diff mbox series

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

Message ID 20240104010144.2137857-1-prasad@arista.com
State Changes Requested
Headers show
Series [iwl-net] igc: write to phy power management and management control registers to power up/power down the phy during interface up/down | expand

Commit Message

Prasad Koya Jan. 4, 2024, 1:01 a.m. UTC 
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.

Signed-off-by: Prasad Koya <prasad@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(-)

Comments

Paul Menzel Jan. 4, 2024, 4:35 p.m. UTC | #1 
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 <prasad@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);
>   }
>   
>   /**
diff mbox series

Patch

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);
 }
 
 /**