Re: [PATCH iwl-next v3] ice: add 200G_AUI8 PHY type definitions and wire them up

[Tony Nguyen <anthony.l.nguyen@intel.com>]

On 3/24/2026 8:35 AM, Aleksandr Loktionov wrote:
> ice_link_mode_str_high[] lacks entries for phy_type_high bits 5-14
> (all 200G PHY types on E825C); ice_dump_phy_type() prints nothing for
> them when ICE_DBG_LINK is set (e.g. 'ethtool -s ethX msglvl 0x10').
> The loop also iterates all 64 bits against a 5-entry array - undefined
> behaviour for any matched bit beyond the end.  Add strings for bits
> 5-14 and guard the loop with ARRAY_SIZE(), falling back to "unknown"
> for unrecognised bits.
> 
> ICE_PHY_TYPE_HIGH_200G_AUI8_AOC_ACC (bit 13) and 200G_AUI8 (bit 14)
> were absent from ice_adminq_cmd.h; ICE_PHY_TYPE_HIGH_MAX_INDEX capped
> at 12 caused ice_update_phy_type() to skip them entirely, leaving both
> invisible to 200G speed requests.  Add the definitions and bump
> MAX_INDEX to 14.

I don't believe support for these two types got implemented so I don't 
think there is need to add them.

> 
> Wire the two new types throughout the driver:
> - ice_get_media_type(): handle all ten 200G phy_type_high values so
>    E825C ports no longer return ICE_MEDIA_UNKNOWN.  AOC_ACC interfaces
>    map to FIBER; bare AUI4/AUI8 to DA with cage, else BACKPLANE
>    (matching existing AUI2/CAUI2 logic); CR4_PAM4 to DA; SR4/FR4/LR4/
>    DR4 to FIBER; KR4_PAM4 to BACKPLANE.
> - ice_get_link_speed_based_on_phy_type(): return ICE_AQ_LINK_SPEED_200GB
>    for both new types so ice_update_phy_type() enables them correctly.
> - phy_type_high_lkup[13,14]: AUI8 is 8-lane 25G-per-lane; no
>    200000baseSR8/CR8 ethtool modes exist yet, so approximate with
>    SR4_Full/CR4_Full - matching AUI4 at indices 11-12.  FIXME once
>    those link modes land upstream.
> - ICE_PHY_TYPE_HIGH_MASK_200G: add bits 13-14 for the minimum-speed
>    floor in ice_mask_min_supported_speeds().
> 
> Suggested-by: Paul Greenwalt <paul.greenwalt@intel.com>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Cc: Simon Horman <horms@kernel.org>
> Cc: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
> v3 -> v4: add ARRAY_SIZE() OOB guard in ice_dump_phy_type(); cover all
>            ten 200G phy_type_high values in ice_get_media_type(); add FIXME
>            to lkup[13..14] for missing SR8/CR8 modes; rename subject
>            fix subject; fix debug enable example (ethtool, not modprobe);
>            add AUI8 speed mapping, lkup[13-14], MASK_200G bits 13-14,
>            and AUI8->SR4/CR4 approximation comment
> v1 -> v2: add ICE_PHY_TYPE_HIGH_MAX_INDEX update
> ---
>   .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  4 ++-
>   drivers/net/ethernet/intel/ice/ice_common.c   | 31 ++++++++++++++++++-
>   drivers/net/ethernet/intel/ice/ice_ethtool.c  |  4 ++-
>   drivers/net/ethernet/intel/ice/ice_ethtool.h  |  8 +++++
>   4 files changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> index 859e9c6..efe985c 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> @@ -1044,7 +1044,9 @@ struct ice_aqc_get_phy_caps {
>   #define ICE_PHY_TYPE_HIGH_200G_KR4_PAM4		BIT_ULL(10)
>   #define ICE_PHY_TYPE_HIGH_200G_AUI4_AOC_ACC	BIT_ULL(11)
>   #define ICE_PHY_TYPE_HIGH_200G_AUI4		BIT_ULL(12)
> -#define ICE_PHY_TYPE_HIGH_MAX_INDEX		12
> +#define ICE_PHY_TYPE_HIGH_200G_AUI8_AOC_ACC	BIT_ULL(13)
> +#define ICE_PHY_TYPE_HIGH_200G_AUI8		BIT_ULL(14)
> +#define ICE_PHY_TYPE_HIGH_MAX_INDEX		14
>   
>   struct ice_aqc_get_phy_caps_data {
>   	__le64 phy_type_low; /* Use values from ICE_PHY_TYPE_LOW_* */
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
> index ce11fea..2f3a268 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -84,6 +84,16 @@ static const char * const ice_link_mode_str_high[] = {
>   	[2] = "100G_CAUI2",
>   	[3] = "100G_AUI2_AOC_ACC",
>   	[4] = "100G_AUI2",
> +	[5] = "200G_CR4_PAM4",
> +	[6] = "200G_SR4",
> +	[7] = "200G_FR4",
> +	[8] = "200G_LR4",
> +	[9] = "200G_DR4",
> +	[10] = "200G_KR4_PAM4",
> +	[11] = "200G_AUI4_AOC_ACC",
> +	[12] = "200G_AUI4",
> +	[13] = "200G_AUI8_AOC_ACC",
> +	[14] = "200G_AUI8",
>   };
>   
>   /**
> @@ -107,9 +117,14 @@ ice_dump_phy_type(struct ice_hw *hw, u64 low, u64 high, const char *prefix)
>   	ice_debug(hw, ICE_DBG_PHY, "%s: phy_type_high: 0x%016llx\n", prefix, high);
>   
>   	for (u32 i = 0; i < BITS_PER_TYPE(typeof(high)); i++) {
> -		if (high & BIT_ULL(i))
> +		if (!(high & BIT_ULL(i)))
> +			continue;
> +		if (i < ARRAY_SIZE(ice_link_mode_str_high))
>   			ice_debug(hw, ICE_DBG_PHY, "%s:   bit(%d): %s\n",
>   				  prefix, i, ice_link_mode_str_high[i]);
> +		else
> +			ice_debug(hw, ICE_DBG_PHY, "%s:   bit(%d): unknown\n",
> +				  prefix, i);
>   	}
>   }
>   
> @@ -605,13 +620,25 @@ static enum ice_media_type ice_get_media_type(struct ice_port_info *pi)
>   		switch (hw_link_info->phy_type_high) {
>   		case ICE_PHY_TYPE_HIGH_100G_AUI2:
>   		case ICE_PHY_TYPE_HIGH_100G_CAUI2:
> +		case ICE_PHY_TYPE_HIGH_200G_AUI4:
> +		case ICE_PHY_TYPE_HIGH_200G_AUI8:
>   			if (ice_is_media_cage_present(pi))
>   				return ICE_MEDIA_DA;
>   			fallthrough;
>   		case ICE_PHY_TYPE_HIGH_100GBASE_KR2_PAM4:
> +		case ICE_PHY_TYPE_HIGH_200G_KR4_PAM4:
>   			return ICE_MEDIA_BACKPLANE;
>   		case ICE_PHY_TYPE_HIGH_100G_CAUI2_AOC_ACC:
>   		case ICE_PHY_TYPE_HIGH_100G_AUI2_AOC_ACC:
> +		case ICE_PHY_TYPE_HIGH_200G_AUI4_AOC_ACC:
> +		case ICE_PHY_TYPE_HIGH_200G_AUI8_AOC_ACC:
> +			return ICE_MEDIA_FIBER;
> +		case ICE_PHY_TYPE_HIGH_200G_CR4_PAM4:
> +			return ICE_MEDIA_DA;

Any reason we don't put this with the other DAs above...

> +		case ICE_PHY_TYPE_HIGH_200G_SR4:
> +		case ICE_PHY_TYPE_HIGH_200G_FR4:
> +		case ICE_PHY_TYPE_HIGH_200G_LR4:
> +		case ICE_PHY_TYPE_HIGH_200G_DR4:
>   			return ICE_MEDIA_FIBER;

and these with the other FIBERs?

Thanks,
Tony