[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f63dca8f-0082-6e22-5ab5-3b940b646053@intel.com>
Date: Fri, 8 Dec 2023 11:07:01 +0100
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Jedrzej Jagielski <jedrzej.jagielski@...el.com>,
<intel-wired-lan@...ts.osuosl.org>
CC: <anthony.l.nguyen@...el.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH iwl-next v3 1/2] ixgbe: Refactor overtemp event handling
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@...el.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]
Powered by blists - more mailing lists