Received-SPF: pass (google.com: domain of linux-kernel+bounces-108040-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <00ec9779-19ce-4005-83f0-f4abf37350fc@arinc9.com>
Date: Tue, 19 Mar 2024 21:46:30 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 0/3] Fix EEE support for MT7531 and MT7988 SoC switch
Content-Language: en-US
To: Daniel Golle <daniel@makrotopia.org>
Cc: DENG Qingfang <dqfext@gmail.com>, Sean Wang <sean.wang@mediatek.com>,
 Andrew Lunn <andrew@lunn.ch>, Florian Fainelli <f.fainelli@gmail.com>,
 Vladimir Oltean <olteanv@gmail.com>, "David S. Miller"
 <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>,
 Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
 Matthias Brugger <matthias.bgg@gmail.com>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>,
 =?UTF-8?Q?Ren=C3=A9_van_Dorst?= <opensource@vdorst.com>,
 Russell King <linux@armlinux.org.uk>,
 SkyLake Huang <SkyLake.Huang@mediatek.com>,
 Heiner Kallweit <hkallweit1@gmail.com>,
 Bartel Eerdekens <bartel.eerdekens@constell8.be>, mithat.guner@xeront.com,
 erkin.bozoglu@xeront.com, netdev@vger.kernel.org,
 linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
 linux-mediatek@lists.infradead.org
References: <<20240318-for-net-mt7530-fix-eee-for-mt7531-mt7988-v>
 <ZfnYkuzuvwLepIfC@makrotopia.org>
From: =?UTF-8?B?QXLEsW7DpyDDnE5BTA==?= <arinc.unal@arinc9.com>
In-Reply-To: <ZfnYkuzuvwLepIfC@makrotopia.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 19.03.2024 21:25, Daniel Golle wrote:
> On Mon, Mar 18, 2024 at 10:46:22AM +0300, Arınç ÜNAL via B4 Relay wrote:
>> Hi.
>>
>> This patch series fixes EEE support for MT7531 and the switch on the MT7988
>> SoC. EEE did not work on MT7531 on most boards before this, it is unclear
>> what's the status on MT7988 SoC switch as I don't have the hardware.
> 
> EEE seems to already work just fine on the MT7988 built-in switch, at least
> on the BPI-R4. I don't think the SoC has bootstrap pins related to EEE like
> stand-alone MT753x may have.
> 
> root@bpi-r4:~# ethtool --show-eee lan1
> EEE settings for lan1:
>      EEE status: disabled
>      Tx LPI: 30 (us)
>      Supported EEE link modes:  100baseT/Full
>                                 1000baseT/Full
>      Advertised EEE link modes:  Not reported
>      Link partner advertised EEE link modes:  100baseT/Full
>                                               1000baseT/Full
> 
> root@bpi-r4:~# ethtool --set-eee lan1 eee on
> root@bpi-r4:~# ethtool --show-eee lan1
> EEE settings for lan1:
>      EEE status: enabled - inactive
>      Tx LPI: 30 (us)
>      Supported EEE link modes:  100baseT/Full
>                                 1000baseT/Full
>      Advertised EEE link modes:  100baseT/Full
>                                  1000baseT/Full
>      Link partner advertised EEE link modes:  Not reported
> root@bpi-r4:~# ethtool --show-eee lan1
> EEE settings for lan1:
>      EEE status: enabled - active
>      Tx LPI: 30 (us)
>      Supported EEE link modes:  100baseT/Full
>                                 1000baseT/Full
>      Advertised EEE link modes:  100baseT/Full
>                                  1000baseT/Full
>      Link partner advertised EEE link modes:  100baseT/Full
>                                               1000baseT/Full
> 
> So don't fix if it ain't broken maybe...?

I would argue that EEE advertisement on the PHY should be enabled by
default. I guess we're supposed to supply that on the PHY driver. Can you
test with this diff applied and see if it works without manually enabling
EEE using ethtool?

diff --git a/drivers/net/phy/mediatek-ge-soc.c b/drivers/net/phy/mediatek-ge-soc.c
index 0f3a1538a8b8..5f482c12018a 100644
--- a/drivers/net/phy/mediatek-ge-soc.c
+++ b/drivers/net/phy/mediatek-ge-soc.c
@@ -978,6 +978,9 @@ static void mt798x_phy_eee(struct phy_device *phydev)
  		       MTK_PHY_RG_LPI_PCS_DSP_CTRL_REG122,
  		       MTK_PHY_LPI_NORM_MSE_HI_THRESH1000_MASK,
  		       FIELD_PREP(MTK_PHY_LPI_NORM_MSE_HI_THRESH1000_MASK, 0xff));
+
+	phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, MDIO_EEE_100TX |
+		      MDIO_EEE_1000T);
  }
  
  static int cal_sw(struct phy_device *phydev, enum CAL_ITEM cal_item,

Arınç