Re: [Intel-wired-lan] [PATCH] [iwl-net] igc: write to phy power management and management control registers to power up/power down the phy during interface up/down

[Paul Menzel]

Dear Prasad,

Thank you very much for your patch. Please remember if you sent an 
updated version to add the version, cf. `--reroll-count|-v` in 
git-format-patch(1). Please also add a small change-log for the patch 
revisions below the --- line.

Also, the git commit message summary (title/subject) is too long. Maybe:

Power PHY up/down on interface up/down

Am 04.01.24 um 02:01 schrieb Prasad Koya:
  For I225/226 parts, when the interface is set down with "ip
  link set <dev> down", interface is down but the PHY (led) continues
  to be up. This patch makes the phy to be actually off/on during
  interface down/up events.

Please do not indent the commit message.

Which datasheet did you use for this? Please mention it. How did you 
test it?

Sorry for these style comments.


Kind regards,

Paul


Signed-off-by: Prasad Koya <pra...@arista.com>
---
  drivers/net/ethernet/intel/igc/igc_defines.h |  3 ++
  drivers/net/ethernet/intel/igc/igc_phy.c     | 42 +++++++++++++-------
  2 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h 
b/drivers/net/ethernet/intel/igc/igc_defines.h
index b3037016f31d..6f60f5bd9cc7 100644
--- a/drivers/net/ethernet/intel/igc/igc_defines.h
+++ b/drivers/net/ethernet/intel/igc/igc_defines.h
@@ -684,4 +684,7 @@
  #define IGC_LTRMAXV_LSNP_REQ          0x00008000 /* LTR Snoop Requirement */
  #define IGC_LTRMAXV_SCALE_SHIFT               10
  
+/* PHY Power management register */
+#define IGC_GO_LINK_DISCONNECT         0x0020     /* Go Link Disconnect */
+
  #endif /* _IGC_DEFINES_H_ */
diff --git a/drivers/net/ethernet/intel/igc/igc_phy.c 
b/drivers/net/ethernet/intel/igc/igc_phy.c
index 53b77c969c85..319cdf876f4e 100644
--- a/drivers/net/ethernet/intel/igc/igc_phy.c
+++ b/drivers/net/ethernet/intel/igc/igc_phy.c
@@ -107,12 +107,22 @@ s32 igc_phy_has_link(struct igc_hw *hw, u32 iterations,
   */
  void igc_power_up_phy_copper(struct igc_hw *hw)
  {
-       u16 mii_reg = 0;
+       struct igc_phy_info *phy = &hw->phy;
+       u32 phpm, manc;
+
+       if (phy->ops.acquire(hw))
+               return;
+
+       manc = rd32(IGC_MANC);
+       manc &= ~IGC_MANC_BLK_PHY_RST_ON_IDE;
+       wr32(IGC_MANC, manc);
  
-	/* 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;
-       hw->phy.ops.write_reg(hw, PHY_CONTROL, mii_reg);
+       phpm = rd32(IGC_I225_PHPM);
+       phpm &= ~IGC_GO_LINK_DISCONNECT;
+       wr32(IGC_I225_PHPM, phpm);
+       usleep_range(100, 200);
+
+       hw->phy.ops.release(hw);
  }
  
  /**
@@ -124,17 +134,21 @@ void igc_power_up_phy_copper(struct igc_hw *hw)
   */
  void igc_power_down_phy_copper(struct igc_hw *hw)
  {
-       u16 mii_reg = 0;
-
-       /* 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;
+       struct igc_phy_info *phy = &hw->phy;
+       u32 phpm, manc;
  
-	/* Temporary workaround - should be removed when PHY will implement
-        * IEEE registers as properly
-        */
-       /* hw->phy.ops.write_reg(hw, PHY_CONTROL, mii_reg);*/
+       if (phy->ops.acquire(hw))
+               return;
+       /* Set "Go Link Disconnect" bit in the PHPM register to turn off the 
PHY */
+       phpm = rd32(IGC_I225_PHPM);
+       phpm |= IGC_GO_LINK_DISCONNECT;
+       wr32(IGC_I225_PHPM, phpm);
        usleep_range(1000, 2000);
+
+       manc = rd32(IGC_MANC);
+       wr32(IGC_MANC, manc | IGC_MANC_BLK_PHY_RST_ON_IDE);
+
+       phy->ops.release(hw);
  }
  
  /**