On 9/6/2025 6:49 AM, Kohei Enju wrote:
On Mon, 1 Sep 2025 07:36:21 +0300, Lifshits, Vitaly wrote:
---
drivers/net/ethernet/intel/igc/igc_ethtool.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c
b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index f3e7218ba6f3..ca93629b1d3a 100644
--- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
+++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
@@ -2094,6 +2094,9 @@ static void igc_ethtool_diag_test(struct net_device
*netdev,
netdev_info(adapter->netdev, "Offline testing starting");
set_bit(__IGC_TESTING, &adapter->state);
+ /* power up PHY for link test */
+ igc_power_up_phy_copper(&adapter->hw);
I suggest moving this to igc_link_test functionn igc_diags.c as it
relates only to the link test.
Thank you for taking a look, Lifshits.
Now I'm considering changing only offline test path, not including
online test path.
This is because I think online test path should not trigger any
interruption and power down/up PHY may cause disruption.
So, I forgo the online path and my intention for this patch is to power
up PHY state only in offline test path.
I think introducing igc_power_up_phy_copper() in igc_link_test()
needs careful consideration in online test path.
Yes, I see that now.
Then it's ok to leave it as is.
Regarding the question whether to wrap igc_power_up_phy_copper with an
if (hw->phy.media_type == igc_media_type_copper), I think that it's not
necessary. First of all, igc devices are only copper devices. Secondly,
in the other place where we call this function (in igc_main), we don't
check the media type, therefore I suggest being consistent with the
already existing code and leaving it as it is now.
+
/* Link test performed before hardware reset so autoneg doesn't
* interfere with test result
*/
Thank you for this patch.
