Received-SPF: pass (google.com: domain of linux-kernel+bounces-77853-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <4a43c21e-1ba9-4853-b2e2-8d2f5f8be464@gmail.com>
Date: Fri, 23 Feb 2024 07:58:17 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v5 2/8] net: phy: Add phydev->enable_tx_lpi to
 simplify adjust link callbacks
To: Wei Fang <wei.fang@nxp.com>, Oleksij Rempel <o.rempel@pengutronix.de>,
 "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>,
 Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
 Andrew Lunn <andrew@lunn.ch>, Russell King <linux@armlinux.org.uk>
Cc: Florian Fainelli <florian.fainelli@broadcom.com>,
 "kernel@pengutronix.de" <kernel@pengutronix.de>,
 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
 "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
 Shenwei Wang <shenwei.wang@nxp.com>, Clark Wang <xiaoning.wang@nxp.com>,
 dl-linux-imx <linux-imx@nxp.com>
References: <20240221062107.778661-1-o.rempel@pengutronix.de>
 <20240221062107.778661-3-o.rempel@pengutronix.de>
 <AM5PR04MB3139BF92ABAE988EAD75088D88552@AM5PR04MB3139.eurprd04.prod.outlook.com>
Content-Language: en-US
From: Heiner Kallweit <hkallweit1@gmail.com>
Autocrypt: addr=hkallweit1@gmail.com; keydata=
 xsFNBF/0ZFUBEAC0eZyktSE7ZNO1SFXL6cQ4i4g6Ah3mOUIXSB4pCY5kQ6OLKHh0FlOD5/5/
 sY7IoIouzOjyFdFPnz4Bl3927ClT567hUJJ+SNaFEiJ9vadI6vZm2gcY4ExdIevYHWe1msJF
 MVE4yNwdS+UsPeCF/6CQQTzHc+n7DomE7fjJD5J1hOJjqz2XWe71fTvYXzxCFLwXXbBiqDC9
 dNqOe5odPsa4TsWZ09T33g5n2nzTJs4Zw8fCy8rLqix/raVsqr8fw5qM66MVtdmEljFaJ9N8
 /W56qGCp+H8Igk/F7CjlbWXiOlKHA25mPTmbVp7VlFsvsmMokr/imQr+0nXtmvYVaKEUwY2g
 86IU6RAOuA8E0J5bD/BeyZdMyVEtX1kT404UJZekFytJZrDZetwxM/cAH+1fMx4z751WJmxQ
 J7mIXSPuDfeJhRDt9sGM6aRVfXbZt+wBogxyXepmnlv9K4A13z9DVLdKLrYUiu9/5QEl6fgI
 kPaXlAZmJsQfoKbmPqCHVRYj1lpQtDM/2/BO6gHASflWUHzwmBVZbS/XRs64uJO8CB3+V3fa
 cIivllReueGCMsHh6/8wgPAyopXOWOxbLsZ291fmZqIR0L5Y6b2HvdFN1Xhc+YrQ8TKK+Z4R
 mJRDh0wNQ8Gm89g92/YkHji4jIWlp2fwzCcx5+lZCQ1XdqAiHQARAQABzSZIZWluZXIgS2Fs
 bHdlaXQgPGhrYWxsd2VpdDFAZ21haWwuY29tPsLBjgQTAQgAOBYhBGxfqY/yOyXjyjJehXLe
 ig9U8DoMBQJf9GRVAhsDBQsJCAcCBhUKCQgLAgQWAgMBAh4BAheAAAoJEHLeig9U8DoMSycQ
 AJbfg8HZEK0ljV4M8nvdaiNixWAufrcZ+SD8zhbxl8GispK4F3Yo+20Y3UoZ7FcIidJWUUJL
 axAOkpI/70YNhlqAPMsuudlAieeYZKjIv1WV5ucNZ3VJ7dC+dlVqQdAr1iD869FZXvy91KhJ
 wYulyCf+s4T9YgmLC6jLMBZghKIf1uhSd0NzjyCqYWbk2ZxByZHgunEShOhHPHswu3Am0ftt
 ePaYIHgZs+Vzwfjs8I7EuW/5/f5G9w1vibXxtGY/GXwgGGHRDjFM7RSprGOv4F5eMGh+NFUJ
 TU9N96PQYMwXVxnQfRXl8O6ffSVmFx4H9rovxWPKobLmqQL0WKLLVvA/aOHCcMKgfyKRcLah
 57vGC50Ga8oT2K1g0AhKGkyJo7lGXkMu5yEs0m9O+btqAB261/E3DRxfI1P/tvDZpLJKtq35
 dXsj6sjvhgX7VxXhY1wE54uqLLHY3UZQlmH3QF5t80MS7/KhxB1pO1Cpcmkt9hgyzH8+5org
 +9wWxGUtJWNP7CppY+qvv3SZtKJMKsxqk5coBGwNkMms56z4qfJm2PUtJQGjA65XWdzQACib
 2iaDQoBqGZfXRdPT0tC1H5kUJuOX4ll1hI/HBMEFCcO8++Bl2wcrUsAxLzGvhINVJX2DAQaF
 aNetToazkCnzubKfBOyiTqFJ0b63c5dqziAgzsFNBF/0ZFUBEADF8UEZmKDl1w/UxvjeyAeX
 kghYkY3bkK6gcIYXdLRfJw12GbvMioSguvVzASVHG8h7NbNjk1yur6AONfbUpXKSNZ0skV8V
 fG+ppbaY+zQofsSMoj5gP0amwbwvPzVqZCYJai81VobefTX2MZM2Mg/ThBVtGyzV3NeCpnBa
 8AX3s9rrX2XUoCibYotbbxx9afZYUFyflOc7kEpc9uJXIdaxS2Z6MnYLHsyVjiU6tzKCiVOU
 KJevqvzPXJmy0xaOVf7mhFSNQyJTrZpLa+tvB1DQRS08CqYtIMxRrVtC0t0LFeQGly6bOngr
 ircurWJiJKbSXVstLHgWYiq3/GmCSx/82ObeLO3PftklpRj8d+kFbrvrqBgjWtMH4WtK5uN5
 1WJ71hWJfNchKRlaJ3GWy8KolCAoGsQMovn/ZEXxrGs1ndafu47yXOpuDAozoHTBGvuSXSZo
 ythk/0EAuz5IkwkhYBT1MGIAvNSn9ivE5aRnBazugy0rTRkVggHvt3/7flFHlGVGpBHxFUwb
 /a4UjJBPtIwa4tWR8B1Ma36S8Jk456k2n1id7M0LQ+eqstmp6Y+UB+pt9NX6t0Slw1NCdYTW
 gJezWTVKF7pmTdXszXGxlc9kTrVUz04PqPjnYbv5UWuDd2eyzGjrrFOsJEi8OK2d2j4FfF++
 AzOMdW09JVqejQARAQABwsF2BBgBCAAgFiEEbF+pj/I7JePKMl6Fct6KD1TwOgwFAl/0ZFUC
 GwwACgkQct6KD1TwOgxUfg//eAoYc0Vm4NrxymfcY30UjHVD0LgSvU8kUmXxil3qhFPS7KA+
 y7tgcKLHOkZkXMX5MLFcS9+SmrAjSBBV8omKoHNo+kfFx/dUAtz0lot8wNGmWb+NcHeKM1eb
 nwUMOEa1uDdfZeKef/U/2uHBceY7Gc6zPZPWgXghEyQMTH2UhLgeam8yglyO+A6RXCh+s6ak
 Wje7Vo1wGK4eYxp6pwMPJXLMsI0ii/2k3YPEJPv+yJf90MbYyQSbkTwZhrsokjQEaIfjrIk3
 rQRjTve/J62WIO28IbY/mENuGgWehRlTAbhC4BLTZ5uYS0YMQCR7v9UGMWdNWXFyrOB6PjSu
 Trn9MsPoUc8qI72mVpxEXQDLlrd2ijEWm7Nrf52YMD7hL6rXXuis7R6zY8WnnBhW0uCfhajx
 q+KuARXC0sDLztcjaS3ayXonpoCPZep2Bd5xqE4Ln8/COCslP7E92W1uf1EcdXXIrx1acg21
 H/0Z53okMykVs3a8tECPHIxnre2UxKdTbCEkjkR4V6JyplTS47oWMw3zyI7zkaadfzVFBxk2
 lo/Tny+FX1Azea3Ce7oOnRUEZtWSsUidtIjmL8YUQFZYm+JUIgfRmSpMFq8JP4VH43GXpB/S
 OCrl+/xujzvoUBFV/cHKjEQYBxo+MaiQa1U54ykM2W4DnHb1UiEf5xDkFd4=
In-Reply-To: <AM5PR04MB3139BF92ABAE988EAD75088D88552@AM5PR04MB3139.eurprd04.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On 23.02.2024 06:36, Wei Fang wrote:
>> -----Original Message-----
>> From: Oleksij Rempel <o.rempel@pengutronix.de>
>> Sent: 2024年2月21日 14:21
>> To: Wei Fang <wei.fang@nxp.com>; David S. Miller <davem@davemloft.net>;
>> Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
>> Paolo Abeni <pabeni@redhat.com>; Andrew Lunn <andrew@lunn.ch>;
>> Heiner Kallweit <hkallweit1@gmail.com>; Russell King
>> <linux@armlinux.org.uk>
>> Cc: Florian Fainelli <florian.fainelli@broadcom.com>; Oleksij Rempel
>> <o.rempel@pengutronix.de>; kernel@pengutronix.de;
>> linux-kernel@vger.kernel.org; netdev@vger.kernel.org; Shenwei Wang
>> <shenwei.wang@nxp.com>; Clark Wang <xiaoning.wang@nxp.com>;
>> dl-linux-imx <linux-imx@nxp.com>
>> Subject: [PATCH net-next v5 2/8] net: phy: Add phydev->enable_tx_lpi to
>> simplify adjust link callbacks
>>
>> From: Andrew Lunn <andrew@lunn.ch>
>>
>> MAC drivers which support EEE need to know the results of the EEE auto-neg
>> in order to program the hardware to perform EEE or not.  The oddly named
>> phy_init_eee() can be used to determine this, it returns 0 if EEE should be
>> used, or a negative error code, e.g. -EOPPROTONOTSUPPORT if the PHY does
>> not support EEE or negotiate resulted in it not being used.
>>
>> However, many MAC drivers get this wrong. Add phydev->enable_tx_lpi
>> which indicates the result of the autoneg for EEE, including if EEE is
>> administratively disabled with ethtool. The MAC driver can then access this in
>> the same way as link speed and duplex in the adjust link callback. If
>> enable_tx_lpi is true, the MAC should send low power indications and does
>> not need to consider anything else with respect to EEE.
>>
>> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
>> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>> ---
>> v2 Check for errors from genphy_c45_eee_is_active
>> v5: Rename to enable_tx_lpi to fit better with phylink changes
>> ---
>>  drivers/net/phy/phy.c | 7 +++++++
>>  include/linux/phy.h   | 2 ++
>>  2 files changed, 9 insertions(+)
>>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index
>> 14224e06d69f..a54b1daf5d5f 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -983,9 +983,16 @@ static int phy_check_link_status(struct phy_device
>> *phydev)
>>  	if (phydev->link && phydev->state != PHY_RUNNING) {
>>  		phy_check_downshift(phydev);
>>  		phydev->state = PHY_RUNNING;
>> +		err = genphy_c45_eee_is_active(phydev,
>> +					       NULL, NULL, NULL);
>> +		if (err < 0)
>> +			phydev->enable_tx_lpi = false;
>> +		else
>> +			phydev->enable_tx_lpi = err;
> 
> phydev->enable_tx_lpi = !!err; Is this better?
> 
I don't think so. Effectively err is a bool value, and it's implicitly
converted.

>>  		phy_link_up(phydev);
>>  	} else if (!phydev->link && phydev->state != PHY_NOLINK) {
>>  		phydev->state = PHY_NOLINK;
>> +		phydev->enable_tx_lpi = false;
>>  		phy_link_down(phydev);
>>  	}
>>
>> diff --git a/include/linux/phy.h b/include/linux/phy.h index
>> e3ab2c347a59..a880f6d7170e 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -594,6 +594,7 @@ struct macsec_ops;
>>   * @supported_eee: supported PHY EEE linkmodes
>>   * @advertising_eee: Currently advertised EEE linkmodes
>>   * @eee_enabled: Flag indicating whether the EEE feature is enabled
>> + * @enable_tx_lpi: When True, MAC should transmit LPI to PHY
>>   * @lp_advertising: Current link partner advertised linkmodes
>>   * @host_interfaces: PHY interface modes supported by host
>>   * @eee_broken_modes: Energy efficient ethernet modes which should be
>> prohibited @@ -713,6 +714,7 @@ struct phy_device {
>>
>>  	/* Energy efficient ethernet modes which should be prohibited */
>>  	u32 eee_broken_modes;
>> +	bool enable_tx_lpi;
>>
>>  #ifdef CONFIG_LED_TRIGGER_PHY
>>  	struct phy_led_trigger *phy_led_triggers;
>> --
>> 2.39.2
>