| Message ID | 20230412133811.2518336-1-arkadiusz.kubalewski@intel.com |
|---|---|
| State | RFC |
| Headers | show |
| Series | [RFC,v1] ice: add CGU info to devlink info callback | expand |
On 4/12/2023 6:38 AM, Arkadiusz Kubalewski wrote: > If Clock Generation Unit and dplls are present on NIC board user shall > know its details. > Provide the devlink info callback with a new: > - fixed type object `cgu.id` - hardware variant of onboard CGU > - running type object `fw.cgu` - CGU firmware version > - running type object `fw.cgu.build` - CGU configuration build version > > These information shall be known for debugging purposes. > > Test (on NIC board with CGU) > $ devlink dev info <bus_name>/<dev_name> | grep cgu > cgu.id 8032 > fw.cgu 6021 > fw.cgu.build 0x1030001 > > Test (on NIC board without CGU) > $ devlink dev info <bus_name>/<dev_name> | grep cgu -c > 0 > > Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> > --- > Documentation/networking/devlink/ice.rst | 14 +++++++++ > drivers/net/ethernet/intel/ice/ice_devlink.c | 30 ++++++++++++++++++++ > drivers/net/ethernet/intel/ice/ice_main.c | 5 +++- > drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 12 ++++---- > drivers/net/ethernet/intel/ice/ice_type.h | 9 +++++- > 5 files changed, 62 insertions(+), 8 deletions(-) > > diff --git a/Documentation/networking/devlink/ice.rst b/Documentation/networking/devlink/ice.rst > index 10f282c2117c..3a54421c503d 100644 > --- a/Documentation/networking/devlink/ice.rst > +++ b/Documentation/networking/devlink/ice.rst > @@ -23,6 +23,11 @@ The ``ice`` driver reports the following versions > - fixed > - K65390-000 > - The Product Board Assembly (PBA) identifier of the board. > + * - ``cgu.id`` > + - fixed > + - 8032 > + - The Clock Generation Unit (CGU) hardware version identifier on the > + board. > * - ``fw.mgmt`` > - running > - 2.1.7 > @@ -89,6 +94,15 @@ The ``ice`` driver reports the following versions > - running > - 0xee16ced7 > - The first 4 bytes of the hash of the netlist module contents. > + * - ``fw.cgu`` > + - running > + - 6021 > + - Version of Clock Generation Unit (CGU) firmware. > + * - ``fw.cgu.build`` > + - running > + - 0x1030001 > + - Version of Clock Generation Unit (CGU) firmware configuration build. > + > > Flash Update > ============ > diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c > index bc44cc220818..06fe895739af 100644 > --- a/drivers/net/ethernet/intel/ice/ice_devlink.c > +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c > @@ -193,6 +193,33 @@ ice_info_pending_netlist_build(struct ice_pf __always_unused *pf, > snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", netlist->hash); > } > > +static void ice_info_cgu_id(struct ice_pf *pf, struct ice_info_ctx *ctx) > +{ > + if (ice_is_feature_supported(pf, ICE_F_CGU)) { > + struct ice_hw *hw = &pf->hw; > + > + snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id); > + } > +} > + > +static void ice_info_cgu_fw_version(struct ice_pf *pf, struct ice_info_ctx *ctx) > +{ > + if (ice_is_feature_supported(pf, ICE_F_CGU)) { > + struct ice_hw *hw = &pf->hw; > + > + snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.fw_ver); > + } > +} > + > +static void ice_info_cgu_fw_build(struct ice_pf *pf, struct ice_info_ctx *ctx) > +{ > + if (ice_is_feature_supported(pf, ICE_F_CGU)) { > + struct ice_hw *hw = &pf->hw; > + > + snprintf(ctx->buf, sizeof(ctx->buf), "0x%x", hw->cgu.cfg_ver); > + } > +} > + Can the CGU values change while the driver is loaded? (i.e. after a firmware update? Does this come as part of the normal firmware block or is it something we have to update separately? Perhaps we want to extract them as part of our preparatory work in the info get handler rather than reading from hw struct. Either way, overall the driver code looks ok. I don't have strong opinions on the naming in devlink info, but I know other folks on the list do. Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > #define fixed(key, getter) { ICE_VERSION_FIXED, key, getter, NULL } > #define running(key, getter) { ICE_VERSION_RUNNING, key, getter, NULL } > #define stored(key, getter, fallback) { ICE_VERSION_STORED, key, getter, fallback } > @@ -224,6 +251,7 @@ static const struct ice_devlink_version { > void (*fallback)(struct ice_pf *pf, struct ice_info_ctx *ctx); > } ice_devlink_versions[] = { > fixed(DEVLINK_INFO_VERSION_GENERIC_BOARD_ID, ice_info_pba), > + fixed("cgu.id", ice_info_cgu_id), > running(DEVLINK_INFO_VERSION_GENERIC_FW_MGMT, ice_info_fw_mgmt), > running("fw.mgmt.api", ice_info_fw_api), > running("fw.mgmt.build", ice_info_fw_build), > @@ -235,6 +263,8 @@ static const struct ice_devlink_version { > running("fw.app.bundle_id", ice_info_ddp_pkg_bundle_id), > combined("fw.netlist", ice_info_netlist_ver, ice_info_pending_netlist_ver), > combined("fw.netlist.build", ice_info_netlist_build, ice_info_pending_netlist_build), > + running("fw.cgu", ice_info_cgu_fw_version), > + running("fw.cgu.build", ice_info_cgu_fw_build), > }; > > /** > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c > index 6b28b95a7254..a3adc03bdd0a 100644 > --- a/drivers/net/ethernet/intel/ice/ice_main.c > +++ b/drivers/net/ethernet/intel/ice/ice_main.c > @@ -4822,8 +4822,11 @@ static void ice_init_features(struct ice_pf *pf) > ice_gnss_init(pf); > > if (ice_is_feature_supported(pf, ICE_F_CGU) || > - ice_is_feature_supported(pf, ICE_F_PHY_RCLK)) > + ice_is_feature_supported(pf, ICE_F_PHY_RCLK)) { > + ice_aq_get_cgu_info(&pf->hw, &pf->hw.cgu.id, > + &pf->hw.cgu.cfg_ver, &pf->hw.cgu.fw_ver); > ice_dpll_init(pf); > + } > > /* Note: Flow director init failure is non-fatal to load */ > if (ice_init_fdir(pf)) > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c > index 39b692945f73..90c1cc1e4401 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c > +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c > @@ -3481,13 +3481,13 @@ bool ice_is_cgu_present(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; > + hw->cgu.part_number = ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032; > return true; > } else if (!ice_find_netlist_node(hw, > ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL, > ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384, > NULL)) { > - hw->cgu_part_number = ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384; > + hw->cgu.part_number = ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384; > return true; > } > > @@ -3507,7 +3507,7 @@ ice_cgu_get_pin_desc_e823(struct ice_hw *hw, bool input, int *size) > { > static const struct ice_cgu_pin_desc *t; > > - if (hw->cgu_part_number == > + if (hw->cgu.part_number == > ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032) { > if (input) { > t = ice_e823_zl_cgu_inputs; > @@ -3516,7 +3516,7 @@ ice_cgu_get_pin_desc_e823(struct ice_hw *hw, bool input, int *size) > t = ice_e823_zl_cgu_outputs; > *size = ARRAY_SIZE(ice_e823_zl_cgu_outputs); > } > - } else if (hw->cgu_part_number == > + } else if (hw->cgu.part_number == > ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384) { > if (input) { > t = ice_e823_si_cgu_inputs; > @@ -3778,10 +3778,10 @@ int ice_get_cgu_rclk_pin_info(struct ice_hw *hw, u8 *base_idx, u8 *pin_num) > case ICE_DEV_ID_E823C_SGMII: > *pin_num = ICE_E822_RCLK_PINS_NUM; > ret = 0; > - if (hw->cgu_part_number == > + if (hw->cgu.part_number == > ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032) > *base_idx = ZL_REF1P; > - else if (hw->cgu_part_number == > + else if (hw->cgu.part_number == > ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384) > *base_idx = SI_REF1P; > else > diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h > index 128bc4d326f9..814166d959ee 100644 > --- a/drivers/net/ethernet/intel/ice/ice_type.h > +++ b/drivers/net/ethernet/intel/ice/ice_type.h > @@ -820,6 +820,13 @@ struct ice_mbx_data { > u16 async_watermark_val; > }; > > +struct ice_cgu_info { > + u32 id; > + u32 cfg_ver; > + u32 fw_ver; > + u8 part_number; > +}; > + > /* Port hardware description */ > struct ice_hw { > u8 __iomem *hw_addr; > @@ -963,7 +970,7 @@ struct ice_hw { > DECLARE_BITMAP(hw_ptype, ICE_FLOW_PTYPE_MAX); > u8 dvm_ena; > u16 io_expander_handle; > - u8 cgu_part_number; > + struct ice_cgu_info cgu; > }; > > /* Statistics collected by each port, VSI, VEB, and S-channel */
On Wed, 12 Apr 2023 15:38:11 +0200 Arkadiusz Kubalewski wrote: > If Clock Generation Unit and dplls are present on NIC board user shall > know its details. > Provide the devlink info callback with a new: > - fixed type object `cgu.id` - hardware variant of onboard CGU > - running type object `fw.cgu` - CGU firmware version > - running type object `fw.cgu.build` - CGU configuration build version > > These information shall be known for debugging purposes. > > Test (on NIC board with CGU) > $ devlink dev info <bus_name>/<dev_name> | grep cgu > cgu.id 8032 > fw.cgu 6021 > fw.cgu.build 0x1030001 > > Test (on NIC board without CGU) > $ devlink dev info <bus_name>/<dev_name> | grep cgu -c > 0 > > Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> Is it flashed together with the rest of the FW components of the NIC? Or the update method is different?
On Wed, Apr 12, 2023 at 03:38:11PM +0200, Arkadiusz Kubalewski wrote: > If Clock Generation Unit and dplls are present on NIC board user shall > know its details. > Provide the devlink info callback with a new: > - fixed type object `cgu.id` - hardware variant of onboard CGU > - running type object `fw.cgu` - CGU firmware version > - running type object `fw.cgu.build` - CGU configuration build version > > These information shall be known for debugging purposes. > > Test (on NIC board with CGU) > $ devlink dev info <bus_name>/<dev_name> | grep cgu > cgu.id 8032 > fw.cgu 6021 > fw.cgu.build 0x1030001 > > Test (on NIC board without CGU) > $ devlink dev info <bus_name>/<dev_name> | grep cgu -c > 0 > > Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> > --- > Documentation/networking/devlink/ice.rst | 14 +++++++++ > drivers/net/ethernet/intel/ice/ice_devlink.c | 30 ++++++++++++++++++++ > drivers/net/ethernet/intel/ice/ice_main.c | 5 +++- > drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 12 ++++---- > drivers/net/ethernet/intel/ice/ice_type.h | 9 +++++- > 5 files changed, 62 insertions(+), 8 deletions(-) <...> > Flash Update > ============ > diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c > index bc44cc220818..06fe895739af 100644 > --- a/drivers/net/ethernet/intel/ice/ice_devlink.c > +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c > @@ -193,6 +193,33 @@ ice_info_pending_netlist_build(struct ice_pf __always_unused *pf, > snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", netlist->hash); > } > > +static void ice_info_cgu_id(struct ice_pf *pf, struct ice_info_ctx *ctx) > +{ > + if (ice_is_feature_supported(pf, ICE_F_CGU)) { > + struct ice_hw *hw = &pf->hw; > + > + snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id); > + } Please use kernel coding style - success oriented flow struct ice_hw *hw = &pf->hw; if (!ice_is_feature_supported(pf, ICE_F_CGU)) return; snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id); However, it will be nice to have these callbacks only if CGU is supported, in such way you won't need any of ice_is_feature_supported() checks. Thanks
>From: Intel-wired-lan intel-wired-lan-bounces@osuosl.org<mailto:intel-wired-lan-bounces@osuosl.org> On Behalf Of >Jacob Keller >Sent: Wednesday, April 12, 2023 11:33 PM > > >On 4/12/2023 6:38 AM, Arkadiusz Kubalewski wrote: >> If Clock Generation Unit and dplls are present on NIC board user shall >> know its details. >> Provide the devlink info callback with a new: >> - fixed type object `cgu.id` - hardware variant of onboard CGU >> - running type object `fw.cgu` - CGU firmware version >> - running type object `fw.cgu.build` - CGU configuration build version >> >> These information shall be known for debugging purposes. >> >> Test (on NIC board with CGU) >> $ devlink dev info <bus_name>/<dev_name> | grep cgu >> cgu.id 8032 >> fw.cgu 6021 >> fw.cgu.build 0x1030001 >> >> Test (on NIC board without CGU) >> $ devlink dev info <bus_name>/<dev_name> | grep cgu -c >> 0 >> >> Signed-off-by: Arkadiusz Kubalewski arkadiusz.kubalewski@intel.com<mailto:arkadiusz.kubalewski@intel.com> >> --- >> Documentation/networking/devlink/ice.rst | 14 +++++++++ >> drivers/net/ethernet/intel/ice/ice_devlink.c | 30 ++++++++++++++++++++ >> drivers/net/ethernet/intel/ice/ice_main.c | 5 +++- >> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 12 ++++---- >> drivers/net/ethernet/intel/ice/ice_type.h | 9 +++++- >> 5 files changed, 62 insertions(+), 8 deletions(-) >> >> diff --git a/Documentation/networking/devlink/ice.rst >>b/Documentation/networking/devlink/ice.rst >> index 10f282c2117c..3a54421c503d 100644 >> --- a/Documentation/networking/devlink/ice.rst >> +++ b/Documentation/networking/devlink/ice.rst >> @@ -23,6 +23,11 @@ The ``ice`` driver reports the following versions >> - fixed >> - K65390-000 >> - The Product Board Assembly (PBA) identifier of the board. >> + * - ``cgu.id`` >> + - fixed >> + - 8032 >> + - The Clock Generation Unit (CGU) hardware version identifier on the >> + board. >> * - ``fw.mgmt`` >> - running >> - 2.1.7 >> @@ -89,6 +94,15 @@ The ``ice`` driver reports the following versions >> - running >> - 0xee16ced7 >> - The first 4 bytes of the hash of the netlist module contents. >> + * - ``fw.cgu`` >> + - running >> + - 6021 >> + - Version of Clock Generation Unit (CGU) firmware. >> + * - ``fw.cgu.build`` >> + - running >> + - 0x1030001 >> + - Version of Clock Generation Unit (CGU) firmware configuration >>build. >> + >> >> Flash Update >> ============ >> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c >>b/drivers/net/ethernet/intel/ice/ice_devlink.c >> index bc44cc220818..06fe895739af 100644 >> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c >> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c >> @@ -193,6 +193,33 @@ ice_info_pending_netlist_build(struct ice_pf >>__always_unused *pf, >> snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", netlist->hash); >> } >> >> +static void ice_info_cgu_id(struct ice_pf *pf, struct ice_info_ctx *ctx) >> +{ >> + if (ice_is_feature_supported(pf, ICE_F_CGU)) { >> + struct ice_hw *hw = &pf->hw; >> + >> + snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id); >> + } >> +} >> + >> +static void ice_info_cgu_fw_version(struct ice_pf *pf, struct >>ice_info_ctx *ctx) >> +{ >> + if (ice_is_feature_supported(pf, ICE_F_CGU)) { >> + struct ice_hw *hw = &pf->hw; >> + >> + snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.fw_ver); >> + } >> +} >> + >> +static void ice_info_cgu_fw_build(struct ice_pf *pf, struct ice_info_ctx >>*ctx) >> +{ >> + if (ice_is_feature_supported(pf, ICE_F_CGU)) { >> + struct ice_hw *hw = &pf->hw; >> + >> + snprintf(ctx->buf, sizeof(ctx->buf), "0x%x", hw->cgu.cfg_ver); >> + } >> +} >> + > >Can the CGU values change while the driver is loaded? (i.e. after a >firmware update? Does this come as part of the normal firmware block or >is it something we have to update separately? Yes, flashing CGU with external programmer could do so, although it is a corner case, it is possible. > >Perhaps we want to extract them as part of our preparatory work in the >info get handler rather than reading from hw struct. Do you mean to ask FW each time we got devlink request from userspace? Well, this seems doable, but do we want to let userspace enquiry FW admin queue without any protection? This smells a bit like a security D.o.S. possibility, I would rather go with reacquiring this info on pf reset or something. As I checked there is already a devlink command: .cmd = DEVLINK_CMD_RELOAD, which reloads this info in ice right now (through ice_load(..)) and it requires .flags = GENL_ADMIN_PERM, looks a bit safer. > >Either way, overall the driver code looks ok. I don't have strong >opinions on the naming in devlink info, but I know other folks on the >list do. > Yeah, hope we can get some answers here :) Thank you! Arkadiusz >Reviewed-by: Jacob Keller jacob.e.keller@intel.com<mailto:jacob.e.keller@intel.com> > >> #define fixed(key, getter) { ICE_VERSION_FIXED, key, getter, NULL } >> #define running(key, getter) { ICE_VERSION_RUNNING, key, getter, NULL } >> #define stored(key, getter, fallback) { ICE_VERSION_STORED, key, getter, >>fallback } >> @@ -224,6 +251,7 @@ static const struct ice_devlink_version { >> void (*fallback)(struct ice_pf *pf, struct ice_info_ctx *ctx); >> } ice_devlink_versions[] = { >> fixed(DEVLINK_INFO_VERSION_GENERIC_BOARD_ID, ice_info_pba), >> + fixed("cgu.id", ice_info_cgu_id), >> running(DEVLINK_INFO_VERSION_GENERIC_FW_MGMT, ice_info_fw_mgmt), >> running("fw.mgmt.api", ice_info_fw_api), >> running("fw.mgmt.build", ice_info_fw_build), >> @@ -235,6 +263,8 @@ static const struct ice_devlink_version { >> running("fw.app.bundle_id", ice_info_ddp_pkg_bundle_id), >> combined("fw.netlist", ice_info_netlist_ver, >>ice_info_pending_netlist_ver), >> combined("fw.netlist.build", ice_info_netlist_build, >>ice_info_pending_netlist_build), >> + running("fw.cgu", ice_info_cgu_fw_version), >> + running("fw.cgu.build", ice_info_cgu_fw_build), >> }; >> >> /** >> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c >>b/drivers/net/ethernet/intel/ice/ice_main.c >> index 6b28b95a7254..a3adc03bdd0a 100644 >> --- a/drivers/net/ethernet/intel/ice/ice_main.c >> +++ b/drivers/net/ethernet/intel/ice/ice_main.c >> @@ -4822,8 +4822,11 @@ static void ice_init_features(struct ice_pf *pf) >> ice_gnss_init(pf); >> >> if (ice_is_feature_supported(pf, ICE_F_CGU) || >> - ice_is_feature_supported(pf, ICE_F_PHY_RCLK)) >> + ice_is_feature_supported(pf, ICE_F_PHY_RCLK)) { >> + ice_aq_get_cgu_info(&pf->hw, &pf->hw.cgu.id, >> + &pf->hw.cgu.cfg_ver, &pf->hw.cgu.fw_ver); >> ice_dpll_init(pf); >> + } >> >> /* Note: Flow director init failure is non-fatal to load */ >> if (ice_init_fdir(pf)) >> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c >>b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c >> index 39b692945f73..90c1cc1e4401 100644 >> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c >> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c >> @@ -3481,13 +3481,13 @@ bool ice_is_cgu_present(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; >> + hw->cgu.part_number = >>ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032; >> return true; >> } else if (!ice_find_netlist_node(hw, >> ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL, >> ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384, >> NULL)) { >> - hw->cgu_part_number = >>ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384; >> + hw->cgu.part_number = >>ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384; >> return true; >> } >> >> @@ -3507,7 +3507,7 @@ ice_cgu_get_pin_desc_e823(struct ice_hw *hw, bool >>input, int *size) >> { >> static const struct ice_cgu_pin_desc *t; >> >> - if (hw->cgu_part_number == >> + if (hw->cgu.part_number == >> ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032) { >> if (input) { >> t = ice_e823_zl_cgu_inputs; >> @@ -3516,7 +3516,7 @@ ice_cgu_get_pin_desc_e823(struct ice_hw *hw, bool >>input, int *size) >> t = ice_e823_zl_cgu_outputs; >> *size = ARRAY_SIZE(ice_e823_zl_cgu_outputs); >> } >> - } else if (hw->cgu_part_number == >> + } else if (hw->cgu.part_number == >> ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384) { >> if (input) { >> t = ice_e823_si_cgu_inputs; >> @@ -3778,10 +3778,10 @@ int ice_get_cgu_rclk_pin_info(struct ice_hw *hw, >>u8 *base_idx, u8 *pin_num) >> case ICE_DEV_ID_E823C_SGMII: >> *pin_num = ICE_E822_RCLK_PINS_NUM; >> ret = 0; >> - if (hw->cgu_part_number == >> + if (hw->cgu.part_number == >> ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032) >> *base_idx = ZL_REF1P; >> - else if (hw->cgu_part_number == >> + else if (hw->cgu.part_number == >> ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384) >> *base_idx = SI_REF1P; >> else >> diff --git a/drivers/net/ethernet/intel/ice/ice_type.h >>b/drivers/net/ethernet/intel/ice/ice_type.h >> index 128bc4d326f9..814166d959ee 100644 >> --- a/drivers/net/ethernet/intel/ice/ice_type.h >> +++ b/drivers/net/ethernet/intel/ice/ice_type.h >> @@ -820,6 +820,13 @@ struct ice_mbx_data { >> u16 async_watermark_val; >> }; >> >> +struct ice_cgu_info { >> + u32 id; >> + u32 cfg_ver; >> + u32 fw_ver; >> + u8 part_number; >> +}; >> + >> /* Port hardware description */ >> struct ice_hw { >> u8 __iomem *hw_addr; >> @@ -963,7 +970,7 @@ struct ice_hw { >> DECLARE_BITMAP(hw_ptype, ICE_FLOW_PTYPE_MAX); >> u8 dvm_ena; >> u16 io_expander_handle; >> - u8 cgu_part_number; >> + struct ice_cgu_info cgu; >> }; >> >> /* Statistics collected by each port, VSI, VEB, and S-channel */ >_______________________________________________ >Intel-wired-lan mailing list >Intel-wired-lan@osuosl.org<mailto:Intel-wired-lan@osuosl.org> >https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
>From: Jakub Kicinski <kuba@kernel.org> >Sent: Thursday, April 13, 2023 5:35 AM > >On Wed, 12 Apr 2023 15:38:11 +0200 Arkadiusz Kubalewski wrote: >> If Clock Generation Unit and dplls are present on NIC board user shall >> know its details. >> Provide the devlink info callback with a new: >> - fixed type object `cgu.id` - hardware variant of onboard CGU >> - running type object `fw.cgu` - CGU firmware version >> - running type object `fw.cgu.build` - CGU configuration build version >> >> These information shall be known for debugging purposes. >> >> Test (on NIC board with CGU) >> $ devlink dev info <bus_name>/<dev_name> | grep cgu >> cgu.id 8032 >> fw.cgu 6021 >> fw.cgu.build 0x1030001 >> >> Test (on NIC board without CGU) >> $ devlink dev info <bus_name>/<dev_name> | grep cgu -c >> 0 >> >> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> > >Is it flashed together with the rest of the FW components of the NIC? >Or the update method is different? Right now there is no mechanics for CGU firmware update at all, this is why I mention that this is for now mostly for debugging purposes. There are already some works ongoing to have CGU FW update possible, first with Intel's nvmupdate packages and tools. But, for Linux we probably also gonna need to support update through devlink, at least this seems right thing to do, as there is already possibility to update NIC firmware with devlink. Thank you! Arkadiusz
>From: Leon Romanovsky <leon@kernel.org> >Sent: Thursday, April 13, 2023 3:17 PM > >On Wed, Apr 12, 2023 at 03:38:11PM +0200, Arkadiusz Kubalewski wrote: >> If Clock Generation Unit and dplls are present on NIC board user shall >> know its details. >> Provide the devlink info callback with a new: >> - fixed type object `cgu.id` - hardware variant of onboard CGU >> - running type object `fw.cgu` - CGU firmware version >> - running type object `fw.cgu.build` - CGU configuration build version >> >> These information shall be known for debugging purposes. >> >> Test (on NIC board with CGU) >> $ devlink dev info <bus_name>/<dev_name> | grep cgu >> cgu.id 8032 >> fw.cgu 6021 >> fw.cgu.build 0x1030001 >> >> Test (on NIC board without CGU) >> $ devlink dev info <bus_name>/<dev_name> | grep cgu -c >> 0 >> >> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> >> --- >> Documentation/networking/devlink/ice.rst | 14 +++++++++ >> drivers/net/ethernet/intel/ice/ice_devlink.c | 30 ++++++++++++++++++++ >> drivers/net/ethernet/intel/ice/ice_main.c | 5 +++- >> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 12 ++++---- >> drivers/net/ethernet/intel/ice/ice_type.h | 9 +++++- >> 5 files changed, 62 insertions(+), 8 deletions(-) > ><...> > >> Flash Update >> ============ >> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c >b/drivers/net/ethernet/intel/ice/ice_devlink.c >> index bc44cc220818..06fe895739af 100644 >> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c >> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c >> @@ -193,6 +193,33 @@ ice_info_pending_netlist_build(struct ice_pf >>__always_unused *pf, >> snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", netlist->hash); >> } >> >> +static void ice_info_cgu_id(struct ice_pf *pf, struct ice_info_ctx *ctx) >> +{ >> + if (ice_is_feature_supported(pf, ICE_F_CGU)) { >> + struct ice_hw *hw = &pf->hw; >> + >> + snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id); >> + } > >Please use kernel coding style - success oriented flow > >struct ice_hw *hw = &pf->hw; > >if (!ice_is_feature_supported(pf, ICE_F_CGU)) > return; > >snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id); > > >However, it will be nice to have these callbacks only if CGU is >supported, in such way you won't need any of ice_is_feature_supported() >checks. > >Thanks Sure, I will fix as suggested in the next version. Although most important is to achieve common understanding and agreement if This way is the right one. Maybe those devlink id's shall be defined as a part of "include/net/devlink.h", so other vendors could use it? Also in such case probably naming might need to be unified. Thank you! Arkadiusz
On Thu, 13 Apr 2023 13:43:52 +0000 Kubalewski, Arkadiusz wrote: > >Is it flashed together with the rest of the FW components of the NIC? > >Or the update method is different? > > Right now there is no mechanics for CGU firmware update at all, this is why I > mention that this is for now mostly for debugging purposes. > There are already some works ongoing to have CGU FW update possible, first with > Intel's nvmupdate packages and tools. But, for Linux we probably also gonna > need to support update through devlink, at least this seems right thing to do, > as there is already possibility to update NIC firmware with devlink. Only FW versions which are updated with a single flashing operation / are part of a single FW image should be reported under the device instance. If the flash is separate you need to create a separate devlink (sub)instance or something along those lines. Differently put - the components in the API are components of the FW image, not components of a board. We had a similar discussion about "line cards" before.
On 4/13/2023 6:36 AM, Kubalewski, Arkadiusz wrote: > Do you mean to ask FW each time we got devlink request from userspace? > > Well, this seems doable, but do we want to let userspace enquiry FW > admin queue > > without any protection? This smells a bit like a security D.o.S. > possibility, > > I would rather go with reacquiring this info on pf reset or something. > > As I checked there is already a devlink command: .cmd = DEVLINK_CMD_RELOAD, > > which reloads this info in ice right now (through ice_load(..)) and it > requires > > .flags = GENL_ADMIN_PERM, looks a bit safer. > > We already do gather some information from firmware at each info request, because we read the NVM to get the inactive flash versions.
On Thu, Apr 13, 2023 at 02:04:33PM +0000, Kubalewski, Arkadiusz wrote: > >From: Leon Romanovsky <leon@kernel.org> > >Sent: Thursday, April 13, 2023 3:17 PM > > > >On Wed, Apr 12, 2023 at 03:38:11PM +0200, Arkadiusz Kubalewski wrote: > >> If Clock Generation Unit and dplls are present on NIC board user shall > >> know its details. > >> Provide the devlink info callback with a new: > >> - fixed type object `cgu.id` - hardware variant of onboard CGU > >> - running type object `fw.cgu` - CGU firmware version > >> - running type object `fw.cgu.build` - CGU configuration build version > >> > >> These information shall be known for debugging purposes. > >> > >> Test (on NIC board with CGU) > >> $ devlink dev info <bus_name>/<dev_name> | grep cgu > >> cgu.id 8032 > >> fw.cgu 6021 > >> fw.cgu.build 0x1030001 > >> > >> Test (on NIC board without CGU) > >> $ devlink dev info <bus_name>/<dev_name> | grep cgu -c > >> 0 > >> > >> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> > >> --- > >> Documentation/networking/devlink/ice.rst | 14 +++++++++ > >> drivers/net/ethernet/intel/ice/ice_devlink.c | 30 ++++++++++++++++++++ > >> drivers/net/ethernet/intel/ice/ice_main.c | 5 +++- > >> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 12 ++++---- > >> drivers/net/ethernet/intel/ice/ice_type.h | 9 +++++- > >> 5 files changed, 62 insertions(+), 8 deletions(-) > > > ><...> > > > >> Flash Update > >> ============ > >> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c > >b/drivers/net/ethernet/intel/ice/ice_devlink.c > >> index bc44cc220818..06fe895739af 100644 > >> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c > >> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c > >> @@ -193,6 +193,33 @@ ice_info_pending_netlist_build(struct ice_pf > >>__always_unused *pf, > >> snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", netlist->hash); > >> } > >> > >> +static void ice_info_cgu_id(struct ice_pf *pf, struct ice_info_ctx *ctx) > >> +{ > >> + if (ice_is_feature_supported(pf, ICE_F_CGU)) { > >> + struct ice_hw *hw = &pf->hw; > >> + > >> + snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id); > >> + } > > > >Please use kernel coding style - success oriented flow > > > >struct ice_hw *hw = &pf->hw; > > > >if (!ice_is_feature_supported(pf, ICE_F_CGU)) > > return; > > > >snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id); > > > > > >However, it will be nice to have these callbacks only if CGU is > >supported, in such way you won't need any of ice_is_feature_supported() > >checks. > > > >Thanks > > Sure, I will fix as suggested in the next version. > Although most important is to achieve common understanding and agreement if > This way is the right one. Maybe those devlink id's shall be defined as a > part of "include/net/devlink.h", so other vendors could use it? Once second vendor materialize, it will be his responsibility to move common code to devlink.h. > Also in such case probably naming might need to be unified. > > Thank you! > Arkadiusz
>From: Jakub Kicinski <kuba@kernel.org> >Sent: Thursday, April 13, 2023 5:04 PM > >On Thu, 13 Apr 2023 13:43:52 +0000 Kubalewski, Arkadiusz wrote: >> >Is it flashed together with the rest of the FW components of the NIC? >> >Or the update method is different? >> >> Right now there is no mechanics for CGU firmware update at all, this is why I >> mention that this is for now mostly for debugging purposes. >> There are already some works ongoing to have CGU FW update possible, first >>with >> Intel's nvmupdate packages and tools. But, for Linux we probably also gonna >> need to support update through devlink, at least this seems right thing to >>do, >> as there is already possibility to update NIC firmware with devlink. > >Only FW versions which are updated with a single flashing operation / >are part of a single FW image should be reported under the device >instance. If the flash is separate you need to create a separate >devlink (sub)instance or something along those lines. > >Differently put - the components in the API are components of the FW >image, not components of a board. > >We had a similar discussion about "line cards" before. Well, this makes sense. Although I double checked, and it seems I wasn't clear on previous explanation. Once FW update is possible with Intel's nvmupdate tools, the devlink FW update also going to update CGU firmware (part of nvm-flash region), so after all this seems a right place for this info. Thanks, Arkadiusz
>From: Leon Romanovsky <leon@kernel.org> >Sent: Thursday, April 13, 2023 7:17 PM > >On Thu, Apr 13, 2023 at 02:04:33PM +0000, Kubalewski, Arkadiusz wrote: >> >From: Leon Romanovsky <leon@kernel.org> >> >Sent: Thursday, April 13, 2023 3:17 PM >> > >> >On Wed, Apr 12, 2023 at 03:38:11PM +0200, Arkadiusz Kubalewski wrote: >> >> If Clock Generation Unit and dplls are present on NIC board user shall >> >> know its details. >> >> Provide the devlink info callback with a new: >> >> - fixed type object `cgu.id` - hardware variant of onboard CGU >> >> - running type object `fw.cgu` - CGU firmware version >> >> - running type object `fw.cgu.build` - CGU configuration build version >> >> >> >> These information shall be known for debugging purposes. >> >> >> >> Test (on NIC board with CGU) >> >> $ devlink dev info <bus_name>/<dev_name> | grep cgu >> >> cgu.id 8032 >> >> fw.cgu 6021 >> >> fw.cgu.build 0x1030001 >> >> >> >> Test (on NIC board without CGU) >> >> $ devlink dev info <bus_name>/<dev_name> | grep cgu -c >> >> 0 >> >> >> >> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> >> >> --- >> >> Documentation/networking/devlink/ice.rst | 14 +++++++++ >> >> drivers/net/ethernet/intel/ice/ice_devlink.c | 30 >> >>++++++++++++++++++++ >> >> drivers/net/ethernet/intel/ice/ice_main.c | 5 +++- >> >> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 12 ++++---- >> >> drivers/net/ethernet/intel/ice/ice_type.h | 9 +++++- >> >> 5 files changed, 62 insertions(+), 8 deletions(-) >> > >> ><...> >> > >> >> Flash Update >> >> ============ >> >> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c >> >b/drivers/net/ethernet/intel/ice/ice_devlink.c >> >> index bc44cc220818..06fe895739af 100644 >> >> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c >> >> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c >> >> @@ -193,6 +193,33 @@ ice_info_pending_netlist_build(struct ice_pf >> >>__always_unused *pf, >> >> snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", netlist-hash); >> >> } >> >> >> >> +static void ice_info_cgu_id(struct ice_pf *pf, struct ice_info_ctx *ctx) >> >> +{ >> >> + if (ice_is_feature_supported(pf, ICE_F_CGU)) { >> >> + struct ice_hw *hw = &pf->hw; >> >> + >> >> + snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id); >> >> + } >> > >> >Please use kernel coding style - success oriented flow >> > >> >struct ice_hw *hw = &pf->hw; >> > >> >if (!ice_is_feature_supported(pf, ICE_F_CGU)) >> > return; >> > >> >snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id); >> > >> > >> >However, it will be nice to have these callbacks only if CGU is >> >supported, in such way you won't need any of ice_is_feature_supported() >> >checks. >> > >> >Thanks >> >> Sure, I will fix as suggested in the next version. >> Although most important is to achieve common understanding and agreement if >> This way is the right one. Maybe those devlink id's shall be defined as a >> part of "include/net/devlink.h", so other vendors could use it? > >Once second vendor materialize, it will be his responsibility to move >common code to devlink.h. > Makes sense, thanks for this explanation! Arkadiusz >> Also in such case probably naming might need to be unified. >> >> Thank you! >> Arkadiusz
>From: Keller, Jacob E <jacob.e.keller@intel.com> >Sent: Thursday, April 13, 2023 6:42 PM > > >On 4/13/2023 6:36 AM, Kubalewski, Arkadiusz wrote: >> Do you mean to ask FW each time we got devlink request from userspace? >> >> Well, this seems doable, but do we want to let userspace enquiry FW >> admin queue >> >> without any protection? This smells a bit like a security D.o.S. >> possibility, >> >> I would rather go with reacquiring this info on pf reset or something. >> >> As I checked there is already a devlink command: .cmd = >DEVLINK_CMD_RELOAD, >> >> which reloads this info in ice right now (through ice_load(..)) and it >> requires >> >> .flags = GENL_ADMIN_PERM, looks a bit safer. >> >> > >We already do gather some information from firmware at each info >request, because we read the NVM to get the inactive flash versions. Your proposal seems simplest solution for fixing the corner case with "externally" programed CGU device (with external programmer on running OS). As I said I can go with this, although once CGU is part of nvm image this gonna look even more corner case, as customer in general shall not mangle the CGU FW on their own. Thank you! Arkadiusz
On Fri, 14 Apr 2023 10:04:05 +0000 Kubalewski, Arkadiusz wrote: > Although I double checked, and it seems I wasn't clear on previous explanation. > Once FW update is possible with Intel's nvmupdate tools, the devlink FW update > also going to update CGU firmware (part of nvm-flash region), so after all this > seems a right place for this info. 👍️
diff --git a/Documentation/networking/devlink/ice.rst b/Documentation/networking/devlink/ice.rst index 10f282c2117c..3a54421c503d 100644 --- a/Documentation/networking/devlink/ice.rst +++ b/Documentation/networking/devlink/ice.rst @@ -23,6 +23,11 @@ The ``ice`` driver reports the following versions - fixed - K65390-000 - The Product Board Assembly (PBA) identifier of the board. + * - ``cgu.id`` + - fixed + - 8032 + - The Clock Generation Unit (CGU) hardware version identifier on the + board. * - ``fw.mgmt`` - running - 2.1.7 @@ -89,6 +94,15 @@ The ``ice`` driver reports the following versions - running - 0xee16ced7 - The first 4 bytes of the hash of the netlist module contents. + * - ``fw.cgu`` + - running + - 6021 + - Version of Clock Generation Unit (CGU) firmware. + * - ``fw.cgu.build`` + - running + - 0x1030001 + - Version of Clock Generation Unit (CGU) firmware configuration build. + Flash Update ============ diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c index bc44cc220818..06fe895739af 100644 --- a/drivers/net/ethernet/intel/ice/ice_devlink.c +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c @@ -193,6 +193,33 @@ ice_info_pending_netlist_build(struct ice_pf __always_unused *pf, snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", netlist->hash); } +static void ice_info_cgu_id(struct ice_pf *pf, struct ice_info_ctx *ctx) +{ + if (ice_is_feature_supported(pf, ICE_F_CGU)) { + struct ice_hw *hw = &pf->hw; + + snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id); + } +} + +static void ice_info_cgu_fw_version(struct ice_pf *pf, struct ice_info_ctx *ctx) +{ + if (ice_is_feature_supported(pf, ICE_F_CGU)) { + struct ice_hw *hw = &pf->hw; + + snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.fw_ver); + } +} + +static void ice_info_cgu_fw_build(struct ice_pf *pf, struct ice_info_ctx *ctx) +{ + if (ice_is_feature_supported(pf, ICE_F_CGU)) { + struct ice_hw *hw = &pf->hw; + + snprintf(ctx->buf, sizeof(ctx->buf), "0x%x", hw->cgu.cfg_ver); + } +} + #define fixed(key, getter) { ICE_VERSION_FIXED, key, getter, NULL } #define running(key, getter) { ICE_VERSION_RUNNING, key, getter, NULL } #define stored(key, getter, fallback) { ICE_VERSION_STORED, key, getter, fallback } @@ -224,6 +251,7 @@ static const struct ice_devlink_version { void (*fallback)(struct ice_pf *pf, struct ice_info_ctx *ctx); } ice_devlink_versions[] = { fixed(DEVLINK_INFO_VERSION_GENERIC_BOARD_ID, ice_info_pba), + fixed("cgu.id", ice_info_cgu_id), running(DEVLINK_INFO_VERSION_GENERIC_FW_MGMT, ice_info_fw_mgmt), running("fw.mgmt.api", ice_info_fw_api), running("fw.mgmt.build", ice_info_fw_build), @@ -235,6 +263,8 @@ static const struct ice_devlink_version { running("fw.app.bundle_id", ice_info_ddp_pkg_bundle_id), combined("fw.netlist", ice_info_netlist_ver, ice_info_pending_netlist_ver), combined("fw.netlist.build", ice_info_netlist_build, ice_info_pending_netlist_build), + running("fw.cgu", ice_info_cgu_fw_version), + running("fw.cgu.build", ice_info_cgu_fw_build), }; /** diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index 6b28b95a7254..a3adc03bdd0a 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -4822,8 +4822,11 @@ static void ice_init_features(struct ice_pf *pf) ice_gnss_init(pf); if (ice_is_feature_supported(pf, ICE_F_CGU) || - ice_is_feature_supported(pf, ICE_F_PHY_RCLK)) + ice_is_feature_supported(pf, ICE_F_PHY_RCLK)) { + ice_aq_get_cgu_info(&pf->hw, &pf->hw.cgu.id, + &pf->hw.cgu.cfg_ver, &pf->hw.cgu.fw_ver); ice_dpll_init(pf); + } /* Note: Flow director init failure is non-fatal to load */ if (ice_init_fdir(pf)) diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c index 39b692945f73..90c1cc1e4401 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c @@ -3481,13 +3481,13 @@ bool ice_is_cgu_present(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; + hw->cgu.part_number = ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032; return true; } else if (!ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL, ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384, NULL)) { - hw->cgu_part_number = ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384; + hw->cgu.part_number = ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384; return true; } @@ -3507,7 +3507,7 @@ ice_cgu_get_pin_desc_e823(struct ice_hw *hw, bool input, int *size) { static const struct ice_cgu_pin_desc *t; - if (hw->cgu_part_number == + if (hw->cgu.part_number == ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032) { if (input) { t = ice_e823_zl_cgu_inputs; @@ -3516,7 +3516,7 @@ ice_cgu_get_pin_desc_e823(struct ice_hw *hw, bool input, int *size) t = ice_e823_zl_cgu_outputs; *size = ARRAY_SIZE(ice_e823_zl_cgu_outputs); } - } else if (hw->cgu_part_number == + } else if (hw->cgu.part_number == ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384) { if (input) { t = ice_e823_si_cgu_inputs; @@ -3778,10 +3778,10 @@ int ice_get_cgu_rclk_pin_info(struct ice_hw *hw, u8 *base_idx, u8 *pin_num) case ICE_DEV_ID_E823C_SGMII: *pin_num = ICE_E822_RCLK_PINS_NUM; ret = 0; - if (hw->cgu_part_number == + if (hw->cgu.part_number == ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032) *base_idx = ZL_REF1P; - else if (hw->cgu_part_number == + else if (hw->cgu.part_number == ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384) *base_idx = SI_REF1P; else diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h index 128bc4d326f9..814166d959ee 100644 --- a/drivers/net/ethernet/intel/ice/ice_type.h +++ b/drivers/net/ethernet/intel/ice/ice_type.h @@ -820,6 +820,13 @@ struct ice_mbx_data { u16 async_watermark_val; }; +struct ice_cgu_info { + u32 id; + u32 cfg_ver; + u32 fw_ver; + u8 part_number; +}; + /* Port hardware description */ struct ice_hw { u8 __iomem *hw_addr; @@ -963,7 +970,7 @@ struct ice_hw { DECLARE_BITMAP(hw_ptype, ICE_FLOW_PTYPE_MAX); u8 dvm_ena; u16 io_expander_handle; - u8 cgu_part_number; + struct ice_cgu_info cgu; }; /* Statistics collected by each port, VSI, VEB, and S-channel */
If Clock Generation Unit and dplls are present on NIC board user shall know its details. Provide the devlink info callback with a new: - fixed type object `cgu.id` - hardware variant of onboard CGU - running type object `fw.cgu` - CGU firmware version - running type object `fw.cgu.build` - CGU configuration build version These information shall be known for debugging purposes. Test (on NIC board with CGU) $ devlink dev info <bus_name>/<dev_name> | grep cgu cgu.id 8032 fw.cgu 6021 fw.cgu.build 0x1030001 Test (on NIC board without CGU) $ devlink dev info <bus_name>/<dev_name> | grep cgu -c 0 Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> --- Documentation/networking/devlink/ice.rst | 14 +++++++++ drivers/net/ethernet/intel/ice/ice_devlink.c | 30 ++++++++++++++++++++ drivers/net/ethernet/intel/ice/ice_main.c | 5 +++- drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 12 ++++---- drivers/net/ethernet/intel/ice/ice_type.h | 9 +++++- 5 files changed, 62 insertions(+), 8 deletions(-)