Dear Sasha,

Thank you for your patch. As always some nits.

Am 24.01.24 um 06:57 schrieb Sasha Neftin:
PHY_CONTROL register works as defined in the IEEE 802.3 specification.

Remove the dot/period as the parenthesis follow?

(IEEE 802.3-2008 22.2.4.1). Tide up the temporary workaround.

I’d be more specific in the commit message summary/title. Maybe (now idea):

igc: Enable PHY_CONTROL

How did you test this?

Fixes: 5586838fe9ce ("igc: Add code for PHY support")
Signed-off-by: Sasha Neftin <[email protected]>
---
  drivers/net/ethernet/intel/igc/igc_phy.c | 6 +-----
  1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_phy.c 
b/drivers/net/ethernet/intel/igc/igc_phy.c
index 7cd8716d2ffa..861f37076861 100644
--- a/drivers/net/ethernet/intel/igc/igc_phy.c
+++ b/drivers/net/ethernet/intel/igc/igc_phy.c
@@ -130,11 +130,7 @@ void igc_power_down_phy_copper(struct igc_hw *hw)
        /* The PHY will retain its settings across a power down/up cycle */
        hw->phy.ops.read_reg(hw, PHY_CONTROL, &mii_reg);
        mii_reg |= MII_CR_POWER_DOWN;
-
-       /* Temporary workaround - should be removed when PHY will implement
-        * IEEE registers as properly
-        */
-       /* hw->phy.ops.write_reg(hw, PHY_CONTROL, mii_reg);*/
+       hw->phy.ops.write_reg(hw, PHY_CONTROL, mii_reg);
        usleep_range(1000, 2000);
  }

Reviewed-by: Paul Menzel <[email protected]>


Kind regards,

Paul

Reply via email to