[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

[Sasha Neftin]

On 04/01/2024 3:01, Prasad Koya wrote:
>   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 at 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);

This register can't written by SW.

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

I prefer to keep aligned with the IEEE PHY access. Using PHPM signals 
could cause uncertain behavior.

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

I believe IEEE std. PHY control access (unmask commented line) should 
work here. (ip link set down <dev>) Have you tried?

> +	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);

The MANC register can be written by the manageability FW and is not 
accessible to the host/SW for writing.

> +
> +	phy->ops.release(hw);
>   }
>   
>   /**