[PATCH iwl-next] ice: unify PHY FW loading status handler for E800 devices

[PATCH iwl-next] ice: unify PHY FW loading status handler for E800 devices

[Grzegorz Nitka]

Unify handling of PHY firmware load delays across all E800 family
devices. There is an existing mechanism to poll GL_MNG_FWSM_FW_LOADING_M
bit of GL_MNG_FWSM register in order to verify whether PHY FW loading
completed or not. Previously, this logic was limited to E827 variants
only.

Also, inform a user of possible delay in initialization process, by
dumping informational message in dmesg log.

Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_common.c | 81 ++++++---------------
 1 file changed, 24 insertions(+), 57 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 8e56354332ad..d05d371a9944 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -203,42 +203,6 @@ bool ice_is_generic_mac(struct ice_hw *hw)
 		hw->mac_type == ICE_MAC_GENERIC_3K_E825);
 }
 
-/**
- * ice_is_pf_c827 - check if pf contains c827 phy
- * @hw: pointer to the hw struct
- *
- * Return: true if the device has c827 phy.
- */
-static bool ice_is_pf_c827(struct ice_hw *hw)
-{
-	struct ice_aqc_get_link_topo cmd = {};
-	u8 node_part_number;
-	u16 node_handle;
-	int status;
-
-	if (hw->mac_type != ICE_MAC_E810)
-		return false;
-
-	if (hw->device_id != ICE_DEV_ID_E810C_QSFP)
-		return true;
-
-	cmd.addr.topo_params.node_type_ctx =
-		FIELD_PREP(ICE_AQC_LINK_TOPO_NODE_TYPE_M, ICE_AQC_LINK_TOPO_NODE_TYPE_PHY) |
-		FIELD_PREP(ICE_AQC_LINK_TOPO_NODE_CTX_M, ICE_AQC_LINK_TOPO_NODE_CTX_PORT);
-	cmd.addr.topo_params.index = 0;
-
-	status = ice_aq_get_netlist_node(hw, &cmd, &node_part_number,
-					 &node_handle);
-
-	if (status || node_part_number != ICE_AQC_GET_LINK_TOPO_NODE_NR_C827)
-		return false;
-
-	if (node_handle == E810C_QSFP_C827_0_HANDLE || node_handle == E810C_QSFP_C827_1_HANDLE)
-		return true;
-
-	return false;
-}
-
 /**
  * ice_clear_pf_cfg - Clear PF configuration
  * @hw: pointer to the hardware structure
@@ -958,30 +922,35 @@ static void ice_get_itr_intrl_gran(struct ice_hw *hw)
 }
 
 /**
- * ice_wait_for_fw - wait for full FW readiness
+ * ice_wait_fw_load - wait for PHY firmware loading to complete
  * @hw: pointer to the hardware structure
  * @timeout: milliseconds that can elapse before timing out
  *
- * Return: 0 on success, -ETIMEDOUT on timeout.
+ * On some cards, FW can load longer than usual,
+ * and could still not be ready before link is turned on.
+ * In these cases, we should wait until all's loaded.
+ *
+ * Return:
+ * * 0 on success (FW load is completed)
+ * * negative - on timeout
  */
-static int ice_wait_for_fw(struct ice_hw *hw, u32 timeout)
+static int ice_wait_fw_load(struct ice_hw *hw, u32 timeout)
 {
-	int fw_loading;
-	u32 elapsed = 0;
+	int fw_loading_reg;
 
-	while (elapsed <= timeout) {
-		fw_loading = rd32(hw, GL_MNG_FWSM) & GL_MNG_FWSM_FW_LOADING_M;
+	if (!timeout)
+		return 0;
 
-		/* firmware was not yet loaded, we have to wait more */
-		if (fw_loading) {
-			elapsed += 100;
-			msleep(100);
-			continue;
-		}
+	fw_loading_reg = rd32(hw, GL_MNG_FWSM) & GL_MNG_FWSM_FW_LOADING_M;
+	/* notify the user only once if PHY FW is still loading */
+	if (fw_loading_reg)
+		dev_info(ice_hw_to_dev(hw), "Link initialization is blocked by PHY FW initialization. Link initialization will continue after PHY FW initialization completes.\n");
+	else
 		return 0;
-	}
 
-	return -ETIMEDOUT;
+	return rd32_poll_timeout(hw, GL_MNG_FWSM, fw_loading_reg,
+				 !(fw_loading_reg & GL_MNG_FWSM_FW_LOADING_M),
+				 10000, timeout * 1000);
 }
 
 static int __fwlog_send_cmd(void *priv, struct libie_aq_desc *desc, void *buf,
@@ -1171,12 +1140,10 @@ int ice_init_hw(struct ice_hw *hw)
 	 * due to necessity of loading FW from an external source.
 	 * This can take even half a minute.
 	 */
-	if (ice_is_pf_c827(hw)) {
-		status = ice_wait_for_fw(hw, 30000);
-		if (status) {
-			dev_err(ice_hw_to_dev(hw), "ice_wait_for_fw timed out");
-			goto err_unroll_fltr_mgmt_struct;
-		}
+	status = ice_wait_fw_load(hw, 30000);
+	if (status) {
+		dev_err(ice_hw_to_dev(hw), "ice_wait_fw_load timed out");
+		goto err_unroll_fltr_mgmt_struct;
 	}
 
 	hw->lane_num = ice_get_phy_lane_number(hw);

base-commit: cbbc9ad6caed63e32e8a4b10001b041f7294ffa6
-- 
2.39.3

Re: [PATCH iwl-next] ice: unify PHY FW loading status handler for E800 devices

[Simon Horman]

On Tue, Oct 14, 2025 at 10:46:18AM +0200, Grzegorz Nitka wrote:
> Unify handling of PHY firmware load delays across all E800 family
> devices. There is an existing mechanism to poll GL_MNG_FWSM_FW_LOADING_M
> bit of GL_MNG_FWSM register in order to verify whether PHY FW loading
> completed or not. Previously, this logic was limited to E827 variants
> only.
> 
> Also, inform a user of possible delay in initialization process, by
> dumping informational message in dmesg log.
> 
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [Intel-wired-lan] [PATCH iwl-next] ice: unify PHY FW loading status handler for E800 devices

[Paul Menzel]

Dear Grzegorz,


Thank you for your patch.

Am 14.10.25 um 10:46 schrieb Grzegorz Nitka:
> Unify handling of PHY firmware load delays across all E800 family
> devices. There is an existing mechanism to poll GL_MNG_FWSM_FW_LOADING_M
> bit of GL_MNG_FWSM register in order to verify whether PHY FW loading
> completed or not. Previously, this logic was limited to E827 variants
> only.
> 
> Also, inform a user of possible delay in initialization process, by
> dumping informational message in dmesg log.

Paste the message here?

> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_common.c | 81 ++++++---------------
>   1 file changed, 24 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
> index 8e56354332ad..d05d371a9944 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -203,42 +203,6 @@ bool ice_is_generic_mac(struct ice_hw *hw)
>   		hw->mac_type == ICE_MAC_GENERIC_3K_E825);
>   }
>   
> -/**
> - * ice_is_pf_c827 - check if pf contains c827 phy
> - * @hw: pointer to the hw struct
> - *
> - * Return: true if the device has c827 phy.
> - */
> -static bool ice_is_pf_c827(struct ice_hw *hw)
> -{
> -	struct ice_aqc_get_link_topo cmd = {};
> -	u8 node_part_number;
> -	u16 node_handle;
> -	int status;
> -
> -	if (hw->mac_type != ICE_MAC_E810)
> -		return false;
> -
> -	if (hw->device_id != ICE_DEV_ID_E810C_QSFP)
> -		return true;
> -
> -	cmd.addr.topo_params.node_type_ctx =
> -		FIELD_PREP(ICE_AQC_LINK_TOPO_NODE_TYPE_M, ICE_AQC_LINK_TOPO_NODE_TYPE_PHY) |
> -		FIELD_PREP(ICE_AQC_LINK_TOPO_NODE_CTX_M, ICE_AQC_LINK_TOPO_NODE_CTX_PORT);
> -	cmd.addr.topo_params.index = 0;
> -
> -	status = ice_aq_get_netlist_node(hw, &cmd, &node_part_number,
> -					 &node_handle);
> -
> -	if (status || node_part_number != ICE_AQC_GET_LINK_TOPO_NODE_NR_C827)
> -		return false;
> -
> -	if (node_handle == E810C_QSFP_C827_0_HANDLE || node_handle == E810C_QSFP_C827_1_HANDLE)
> -		return true;
> -
> -	return false;
> -}
> -
>   /**
>    * ice_clear_pf_cfg - Clear PF configuration
>    * @hw: pointer to the hardware structure
> @@ -958,30 +922,35 @@ static void ice_get_itr_intrl_gran(struct ice_hw *hw)
>   }
>   
>   /**
> - * ice_wait_for_fw - wait for full FW readiness
> + * ice_wait_fw_load - wait for PHY firmware loading to complete
>    * @hw: pointer to the hardware structure
>    * @timeout: milliseconds that can elapse before timing out
>    *
> - * Return: 0 on success, -ETIMEDOUT on timeout.
> + * On some cards, FW can load longer than usual,
> + * and could still not be ready before link is turned on.
> + * In these cases, we should wait until all's loaded.
> + *
> + * Return:
> + * * 0 on success (FW load is completed)
> + * * negative - on timeout
>    */
> -static int ice_wait_for_fw(struct ice_hw *hw, u32 timeout)
> +static int ice_wait_fw_load(struct ice_hw *hw, u32 timeout)
>   {
> -	int fw_loading;
> -	u32 elapsed = 0;
> +	int fw_loading_reg;
>   
> -	while (elapsed <= timeout) {
> -		fw_loading = rd32(hw, GL_MNG_FWSM) & GL_MNG_FWSM_FW_LOADING_M;
> +	if (!timeout)
> +		return 0;
>   
> -		/* firmware was not yet loaded, we have to wait more */
> -		if (fw_loading) {
> -			elapsed += 100;
> -			msleep(100);
> -			continue;
> -		}
> +	fw_loading_reg = rd32(hw, GL_MNG_FWSM) & GL_MNG_FWSM_FW_LOADING_M;
> +	/* notify the user only once if PHY FW is still loading */
> +	if (fw_loading_reg)
> +		dev_info(ice_hw_to_dev(hw), "Link initialization is blocked by PHY FW initialization. Link initialization will continue after PHY FW initialization completes.\n");
> +	else
>   		return 0;
> -	}
>   
> -	return -ETIMEDOUT;
> +	return rd32_poll_timeout(hw, GL_MNG_FWSM, fw_loading_reg,
> +				 !(fw_loading_reg & GL_MNG_FWSM_FW_LOADING_M),
> +				 10000, timeout * 1000);
>   }
>   
>   static int __fwlog_send_cmd(void *priv, struct libie_aq_desc *desc, void *buf,
> @@ -1171,12 +1140,10 @@ int ice_init_hw(struct ice_hw *hw)
>   	 * due to necessity of loading FW from an external source.
>   	 * This can take even half a minute.
>   	 */
> -	if (ice_is_pf_c827(hw)) {
> -		status = ice_wait_for_fw(hw, 30000);
> -		if (status) {
> -			dev_err(ice_hw_to_dev(hw), "ice_wait_for_fw timed out");
> -			goto err_unroll_fltr_mgmt_struct;
> -		}
> +	status = ice_wait_fw_load(hw, 30000);
> +	if (status) {
> +		dev_err(ice_hw_to_dev(hw), "ice_wait_fw_load timed out");
> +		goto err_unroll_fltr_mgmt_struct;
>   	}
>   
>   	hw->lane_num = ice_get_phy_lane_number(hw);

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

RE: [Intel-wired-lan] [PATCH iwl-next] ice: unify PHY FW loading status handler for E800 devices

[Nitka, Grzegorz]

> -----Original Message-----
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Sent: Thursday, October 16, 2025 5:54 PM
> To: Nitka, Grzegorz <grzegorz.nitka@intel.com>
> Cc: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; intel-wired-
> lan@lists.osuosl.org; netdev@vger.kernel.org
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next] ice: unify PHY FW loading
> status handler for E800 devices
> 
> Dear Grzegorz,
> 
> 
> Thank you for your patch.
> 
> Am 14.10.25 um 10:46 schrieb Grzegorz Nitka:
> > Unify handling of PHY firmware load delays across all E800 family
> > devices. There is an existing mechanism to poll
> GL_MNG_FWSM_FW_LOADING_M
> > bit of GL_MNG_FWSM register in order to verify whether PHY FW loading
> > completed or not. Previously, this logic was limited to E827 variants
> > only.
> >
> > Also, inform a user of possible delay in initialization process, by
> > dumping informational message in dmesg log.
> 
> Paste the message here?
> 

Thanks Paul for your feedback. Your comment has been addressed in v2.
Kind Regards

Grzegorz

...

> >
> >   	hw->lane_num = ice_get_phy_lane_number(hw);
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> 
> 
> Kind regards,
> 
> Paul