Re: [PATCH v2 1/6] net/ice/base: add utility functions

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Aug 11, 2025 at 01:42:56PM +0000, Vladimir Medvedkin wrote:
> Add the following function related to DCB:
> 
> * Get number of Congestion Domains per port
> * Resolve Traffic Class(TC) by 802.1p VLAN User Priority(UP)
> * Get absolute index of the Congestion Domain by the TC
> 
> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> ---
>  drivers/net/intel/ice/base/ice_common.c | 28 +++++++++++++++++++++++++
>  drivers/net/intel/ice/base/ice_common.h |  8 +++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/net/intel/ice/base/ice_common.c b/drivers/net/intel/ice/base/ice_common.c
> index dfc2fd69e7..8bca256b34 100644
> --- a/drivers/net/intel/ice/base/ice_common.c
> +++ b/drivers/net/intel/ice/base/ice_common.c
> @@ -6579,3 +6579,31 @@ bool ice_is_fw_auto_drop_supported(struct ice_hw *hw)
>  		return true;
>  	return false;
>  }
> +
> +enum ice_cgd_per_port
> +ice_get_port_max_cgd(struct ice_hw *hw)
> +{
> +#define ICE_8_PORTS_LINK_TOPO	0x2
> +	u32 link_topo = LE32_TO_CPU(rd32(hw, GLGEN_MAC_LINK_TOPO)) &
> +		GLGEN_MAC_LINK_TOPO_LINK_TOPO_M;
> +
> +	return (link_topo == ICE_8_PORTS_LINK_TOPO) ? ICE_4_CGD_PER_PORT : ICE_8_CGD_PER_PORT;
> +}

The term "cgd" is not very clear (at least to me) on reading this. I think
a comment at the start of the function to say what it does would be
helpful, e.g. "/* get the number of congestion domains (cgds) for a port */

> +
> +u8
> +ice_get_tc_by_up(struct ice_hw *hw, u8 up)
> +{
> +	struct ice_port_info *port_info = hw->port_info;
> +	struct ice_qos_cfg *qos_cfg = &port_info->qos_cfg;
> +	struct ice_dcbx_cfg *local_dcb_conf = &qos_cfg->local_dcbx_cfg;
> +
> +	up &= (ICE_MAX_TRAFFIC_CLASS - 1);
> +
> +	return local_dcb_conf->etscfg.prio_table[up];
> +}

Rather than "up" (which I assume is "user priority"), I think
"tc_by_priority" would be a better title. Again, a one-line comment at top
of the function can clarify that it's the user-priority field being
referred to.

> +
> +int
> +ice_get_cgd_idx(struct ice_hw *hw, u8 tc)
> +{
> +	return hw->port_info->lport * ice_get_port_max_cgd(hw) + tc;
> +}
Add comment here too - e.g. /* get congestion domain for traffic class */

> diff --git a/drivers/net/intel/ice/base/ice_common.h b/drivers/net/intel/ice/base/ice_common.h
> index f7e7ff5aaf..2d4f4871e6 100644
> --- a/drivers/net/intel/ice/base/ice_common.h
> +++ b/drivers/net/intel/ice/base/ice_common.h
> @@ -25,6 +25,11 @@ enum ice_fw_modes {
>  	ICE_FW_MODE_ROLLBACK
>  };
>  
> +enum ice_cgd_per_port {
> +	ICE_4_CGD_PER_PORT = 4,
> +	ICE_8_CGD_PER_PORT = 8
> +};
> +
>  int ice_init_fltr_mgmt_struct(struct ice_hw *hw);
>  void ice_cleanup_fltr_mgmt_struct(struct ice_hw *hw);
>  void ice_set_umac_shared(struct ice_hw *hw);
> @@ -335,4 +340,7 @@ ice_aq_write_i2c(struct ice_hw *hw, struct ice_aqc_link_topo_addr topo_addr,
>  bool ice_fw_supports_report_dflt_cfg(struct ice_hw *hw);
>  /* AQ API version for FW auto drop reports */
>  bool ice_is_fw_auto_drop_supported(struct ice_hw *hw);
> +enum ice_cgd_per_port ice_get_port_max_cgd(struct ice_hw *hw);
> +u8 ice_get_tc_by_up(struct ice_hw *hw, u8 up);
> +int ice_get_cgd_idx(struct ice_hw *hw, u8 tc);
>  #endif /* _ICE_COMMON_H_ */
> -- 
> 2.43.0
>