Re: [PATCH net-next 1/4] bnxt_en: Change FW message timeout warning

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 15 Apr 2025 10:48:15 -0700 Michael Chan wrote:
> The firmware advertises a "hwrm_cmd_max_timeout" value to the driver
> for NVRAM and coredump related functions that can take tens of seconds
> to complete.  The driver polls for the operation to complete under
> mutex and may trigger hung task watchdog warning if the wait is too long.
> To warn the user about this, the driver currently prints a warning if
> this advertised value exceeds 40 seconds:
> 
> Device requests max timeout of %d seconds, may trigger hung task watchdog
> 
> The original hope was that newer FW would be able to reduce this timeout
> below 40 seconds.  But 60 seconds is the timeout on most production FW
> and cannot be reduced further.  Change the driver's warning threshold to
> 60 seconds to avoid triggering this warning on all production devices.
> 
> Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
> Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.h
> index 15ca51b5d204..fb5f5b063c3d 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.h
> @@ -58,7 +58,7 @@ void hwrm_update_token(struct bnxt *bp, u16 seq, enum bnxt_hwrm_wait_state s);
>  
>  #define BNXT_HWRM_MAX_REQ_LEN		(bp->hwrm_max_req_len)
>  #define BNXT_HWRM_SHORT_REQ_LEN		sizeof(struct hwrm_short_input)
> -#define HWRM_CMD_MAX_TIMEOUT		40000U
> +#define HWRM_CMD_MAX_TIMEOUT		60000U
>  #define SHORT_HWRM_CMD_TIMEOUT		20
>  #define HWRM_CMD_TIMEOUT		(bp->hwrm_cmd_timeout)
>  #define HWRM_RESET_TIMEOUT		((HWRM_CMD_TIMEOUT) * 4)

sysctl_hung_task_timeout_secs is an exported symbol, and it defaults to 120.
Should you not use it in the warning (assuming I understand the intent
there)?