On Sat, 30 Aug 2025 21:27:33 +0200, Andrew Lunn wrote:
>On Sun, Aug 31, 2025 at 02:06:19AM +0900, Kohei Enju wrote:
>> The current implementation of igc driver doesn't power up PHY before
>> link test in igc_ethtool_diag_test(), causing the link test to always
>> report FAIL when admin state is down and PHY is consequently powered
>> down.
>>
>> To test the link state regardless of admin state, let's power up PHY in
>> case of PHY down before link test.
>
>Maybe you should power the PHY down again after the test?
You're right about the concern, but it's already handled by the existing
code flow:
/* power up PHY for link test */
igc_power_up_phy_copper(&adapter->hw);
/* doing link test */
if (if_running)
igc_close(netdev);
else
igc_reset(adapter);
/* other tests */
igc_reset(adapter);
igc_reset() calls igc_power_down_phy_copper_base() when !netif_running(),
so the PHY is properly powered down again.
>
>Alternatively, just return -ENOTDOWN is the network is admin down.
That would be simpler indeed. Since the callback returns void, we'd set
the test result to indicate skip/fail.
However, I think checking actual physical connectivity even when admin
down is valuable, which other Intel ethernet drivers (e.g., i40e, ixgbe,
igb) also do.
>
> Andrew
