From: Nguyen, Anthony L <[email protected]> 
Sent: Monday, December 11, 2023 10:35 PM

>On 12/11/2023 1:45 AM, Jagielski, Jedrzej wrote:
>> From: Kitszel, Przemyslaw <[email protected]>
>> 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 <[email protected]>
>>>> ---
>>>> 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?
>
>We shouldn't carry a return value only to align with other ops. If we 

Sure, just thought it is standardized some way in that case.

>there's no need for it, we shouldn't have it... however, aren't you 
>using/checking it here?

actually there is no need since just overtemp indication is checked

>
>@@ -406,9 +407,12 @@  s32 ixgbe_reset_phy_generic(struct ixgbe_hw *hw)
>       if (status != 0 || hw->phy.type == ixgbe_phy_none)
>               return status;
>
>+      status = hw->phy.ops.check_overtemp(hw, &overtemp);
>+      if (status)
>+              return status;
>
>Thanks,
>Tony

Thanks
Jedrek
_______________________________________________
Intel-wired-lan mailing list
[email protected]
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Reply via email to