From: Kitszel, Przemyslaw <przemyslaw.kits...@intel.com> 
Sent: Friday, December 8, 2023 11:07 AM

>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.jagiel...@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

Generally I think it is good think to give a possibility to return error code,
despite here it is not used (no possibility to handle it since called from
void function, just return).
All other phy ops are also s32 type, so this one is aligned with them.

@Nguyen, Anthony L What do you think on that solution?

>
>> +            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]
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Reply via email to