Re: [PATCH iwl-next v3 1/2] ixgbe: Refactor overtemp event handling

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 12/8/23 10:00, Jedrzej Jagielski wrote:
> Currently ixgbe driver is notified of overheating events
> via internal IXGBE_ERR_OVERTEMP error code.
> 
> Change the approach to use freshly introduced is_overtemp
> function parameter which set when such event occurs.
> Add new parameter to the check_overtemp() and handle_lasi()
> phy ops.
> 
> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
> ---
> v2: change aproach to use additional function parameter to notify when overheat

on public mailing lists its best to require links to previous versions

> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 20 ++++----
>   drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 33 +++++++++----
>   drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |  2 +-
>   drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |  4 +-
>   drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 47 ++++++++++++-------
>   5 files changed, 67 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 227415d61efc..f6200f0d1e06 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -2756,7 +2756,7 @@ static void ixgbe_check_overtemp_subtask(struct ixgbe_adapter *adapter)
>   {
>   	struct ixgbe_hw *hw = &adapter->hw;
>   	u32 eicr = adapter->interrupt_event;
> -	s32 rc;
> +	bool overtemp;
>   
>   	if (test_bit(__IXGBE_DOWN, &adapter->state))
>   		return;
> @@ -2790,14 +2790,15 @@ static void ixgbe_check_overtemp_subtask(struct ixgbe_adapter *adapter)
>   		}
>   
>   		/* Check if this is not due to overtemp */
> -		if (hw->phy.ops.check_overtemp(hw) != IXGBE_ERR_OVERTEMP)
> +		hw->phy.ops.check_overtemp(hw, &overtemp);

you newer (at least in the scope of this patch) check return code of
.check_overtemp(), so you could perhaps instead change it to return
bool, and just return "true if overtemp detected"?

> +		if (!overtemp)
>   			return;
>   
>   		break;
>   	case IXGBE_DEV_ID_X550EM_A_1G_T:
>   	case IXGBE_DEV_ID_X550EM_A_1G_T_L:
> -		rc = hw->phy.ops.check_overtemp(hw);
> -		if (rc != IXGBE_ERR_OVERTEMP)
> +		hw->phy.ops.check_overtemp(hw, &overtemp);
> +		if (!overtemp)
>   			return;
>   		break;
>   	default:
> @@ -2807,6 +2808,7 @@ static void ixgbe_check_overtemp_subtask(struct ixgbe_adapter *adapter)
>   			return;
>   		break;
>   	}
> +

I would remove chunks that are whitespace only

>   	e_crit(drv, "%s\n", ixgbe_overheat_msg);
>   
>   	adapter->interrupt_event = 0;
> @@ -7938,7 +7940,7 @@ static void ixgbe_service_timer(struct timer_list *t)

[snip]