Dear Aleksandr,

Thank you for your patch.

Am 24.03.26 um 16:35 schrieb Aleksandr Loktionov:
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.

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 <[email protected]>
Signed-off-by: Aleksandr Loktionov <[email protected]>
Cc: Simon Horman <[email protected]>
Cc: Paul Menzel <[email protected]>
---
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;
+               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;
                }
        }
@@ -3493,6 +3520,8 @@ u16 ice_get_link_speed_based_on_phy_type(u64 
phy_type_low, u64 phy_type_high)
        case ICE_PHY_TYPE_HIGH_200G_KR4_PAM4:
        case ICE_PHY_TYPE_HIGH_200G_AUI4_AOC_ACC:
        case ICE_PHY_TYPE_HIGH_200G_AUI4:
+       case ICE_PHY_TYPE_HIGH_200G_AUI8_AOC_ACC:
+       case ICE_PHY_TYPE_HIGH_200G_AUI8:
                speed_phy_type_high = ICE_AQ_LINK_SPEED_200GB;
                break;
        default:
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c 
b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index 301947d..beb638c 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -2057,7 +2057,9 @@ ice_get_ethtool_stats(struct net_device *netdev,
                                         ICE_PHY_TYPE_HIGH_200G_DR4 | \
                                         ICE_PHY_TYPE_HIGH_200G_KR4_PAM4 | \
                                         ICE_PHY_TYPE_HIGH_200G_AUI4_AOC_ACC | \
-                                        ICE_PHY_TYPE_HIGH_200G_AUI4)
+                                        ICE_PHY_TYPE_HIGH_200G_AUI4 | \
+                                        ICE_PHY_TYPE_HIGH_200G_AUI8_AOC_ACC | \
+                                        ICE_PHY_TYPE_HIGH_200G_AUI8)
/**
   * ice_mask_min_supported_speeds
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.h 
b/drivers/net/ethernet/intel/ice/ice_ethtool.h
index 23b2cfb..c4732a3 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.h
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.h
@@ -153,6 +153,14 @@ phy_type_high_lkup[] = {
        [10] = ICE_PHY_TYPE(200GB, 200000baseKR4_Full),
        [11] = ICE_PHY_TYPE(200GB, 200000baseSR4_Full),
        [12] = ICE_PHY_TYPE(200GB, 200000baseCR4_Full),
+       /* 200G_AUI8_AOC_ACC and 200G_AUI8 are 8-lane 25G-per-lane interfaces.
+        * The kernel has no 200000baseSR8/CR8 modes yet; map to the closest
+        * available 4-lane equivalents so ethtool reports 200G as supported.
+        * FIXME: replace with 200000baseSR8_Full / 200000baseCR8_Full once
+        * those ethtool link modes are defined upstream.
+        */
+       [13] = ICE_PHY_TYPE(200GB, 200000baseSR4_Full),
+       [14] = ICE_PHY_TYPE(200GB, 200000baseCR4_Full),
  };
#endif /* !_ICE_ETHTOOL_H_ */

Reviewed-by: Paul Menzel <[email protected]>


Kind regards,

Paul

Reply via email to