Re: [PATCH net-next 2/9] eth: fbnic: use fw uptime to detect fw crashes

[Simon Horman <horms@kernel.org>]

On Fri, Sep 12, 2025 at 01:14:21PM -0700, Jakub Kicinski wrote:
> Currently we only detect FW crashes when it stops responding
> to heartbeat messages. FW has a watchdog which will reset it
> in case of crashes. Use FW uptime sent in the heartbeat messages
> to detect that the watchdog has fired.

I see that the time sent in heartbeat messages is critical to this check.
But the time sent in ownership messages is also used, right?

> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  drivers/net/ethernet/meta/fbnic/fbnic.h    |  4 ++++
>  drivers/net/ethernet/meta/fbnic/fbnic_fw.h |  6 ++++++
>  drivers/net/ethernet/meta/fbnic/fbnic_fw.c | 15 ++++++++++++++-
>  3 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
> index 311c7dda911a..09058d847729 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
> @@ -84,6 +84,10 @@ struct fbnic_dev {
>  	/* Local copy of hardware statistics */
>  	struct fbnic_hw_stats hw_stats;
>  
> +	/* Firmware time since boot in milliseconds */
> +	u64 firmware_time;
> +	u64 prev_firmware_time;
> +
>  	struct fbnic_fw_log fw_log;
>  };
>  
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
> index ec67b80809b0..c1de2793fa3d 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
> @@ -201,6 +201,12 @@ enum {
>  	FBNIC_FW_OWNERSHIP_MSG_MAX
>  };
>  
> +enum {
> +	FBNIC_FW_HEARTBEAT_UPTIME               = 0x0,
> +	FBNIC_FW_HEARTBEAT_NUMBER_OF_MESSAGES   = 0x1,
> +	FBNIC_FW_HEARTBEAT_MSG_MAX
> +};
> +
>  enum {
>  	FBNIC_FW_START_UPGRADE_ERROR		= 0x0,
>  	FBNIC_FW_START_UPGRADE_SECTION		= 0x1,
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> index 6e580654493c..72f750eea055 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> @@ -495,6 +495,11 @@ int fbnic_fw_xmit_ownership_msg(struct fbnic_dev *fbd, bool take_ownership)
>  
>  	fbd->last_heartbeat_request = req_time;
>  
> +	/* Set prev_firmware_time to 0 to avoid triggering firmware crash
> +	 * detection now that we received a response from firmware.
> +	 */

I'm having a bit of trouble understanding this comment.
Here we are sending an ownership message.
Have we (also) received a response from firmware?

> +	fbd->prev_firmware_time = 0;
> +
>  	/* Set heartbeat detection based on if we are taking ownership */
>  	fbd->fw_heartbeat_enabled = take_ownership;
>  
> @@ -671,6 +676,9 @@ static int fbnic_fw_parse_ownership_resp(void *opaque,
>  	/* Count the ownership response as a heartbeat reply */
>  	fbd->last_heartbeat_response = jiffies;
>  
> +	/* Capture firmware time for logging and firmware crash check */
> +	fbd->firmware_time = fta_get_uint(results, FBNIC_FW_HEARTBEAT_UPTIME);

Maybe FBNIC_FW_OWNERSHIP_UPTIME?

> +
>  	return 0;
>  }
>  
> @@ -685,6 +693,9 @@ static int fbnic_fw_parse_heartbeat_resp(void *opaque,
>  
>  	fbd->last_heartbeat_response = jiffies;
>  
> +	/* Capture firmware time for logging and firmware crash check */
> +	fbd->firmware_time = fta_get_uint(results, FBNIC_FW_HEARTBEAT_UPTIME);
> +
>  	return 0;
>  }

...