-----Original Message----- From: Heiner Kallweit hkallweit1@gmail.com Sent: Monday, January 10, 2022 6:17 PM To: Ismail, Mohammad Athari mohammad.athari.ismail@intel.com; Andrew Lunn andrew@lunn.ch; David S . Miller davem@davemloft.net; Jakub Kicinski kuba@kernel.org; Oleksij Rempel <linux@rempel- privat.de>; Russell King linux@armlinux.org.uk Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org Subject: Re: [PATCH net 1/1] net: phy: marvell: add Marvell specific PHY loopback
On 10.01.2022 10:36, Ismail, Mohammad Athari wrote:
-----Original Message----- From: Heiner Kallweit hkallweit1@gmail.com Sent: Monday, January 10, 2022 4:34 PM To: Ismail, Mohammad Athari mohammad.athari.ismail@intel.com; Andrew Lunn andrew@lunn.ch; David S . Miller
Jakub Kicinski kuba@kernel.org; Oleksij Rempel <linux@rempel- privat.de>; Russell King linux@armlinux.org.uk Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org Subject: Re: [PATCH net 1/1] net: phy: marvell: add Marvell specific PHY loopback
On 10.01.2022 07:21, Mohammad Athari Bin Ismail wrote:
Existing genphy_loopback() is not applicable for Marvell PHY. So, adding Marvell specific PHY loopback operation by only setting(enable) or clearing(disable) BMCR_LOOPBACK bit.
Tested working on Marvell 88E1510.
With this change you'd basically revert the original change and loose its functionality. Did you check the Marvell datasheets? At least for few versions I found that you may have to configure bits 0..2 in MAC Specific Control Register 2 (page 2, register 21) instead of
BMCR.
May I know what datasheet version that has the bits 2:0's detail
explanation? The version that I have, bits 2:0 in MAC Specific Control Register 2 shows as Reserved.
The datasheet I have is "Marvell Alaska 88E1510/88E1518/88E1512/88E1514
Integrated 10/100/1000 Mbps Energy Efficient Ethernet Transceiver Rev. G December 17, 2021"
I checked the 88E6352 switch chip datasheet. The part covering the integrated PHY's lists the mentioned bits in MAC Specific Control Register 2.
Table 75 in the 88E1510 datasheet says: Loopback speed is determined by Registers 21_2.6,13. So Marvell PHY's seem to use different bits (although same register) for loopback speed configuration.
I get your point. Registers 21_2.6,13 also need to be configured for loopback to work for all speeds. I'll send the v2 patch later.
-Athari-
Really appreciate if you could advice on PHY loopback enabling for Marvell
88E1510 because the existing genphy_loopback() function doesn't work for the PHY.
Thank you.
-Athari-
Fixes: 014068dcb5b1 ("net: phy: genphy_loopback: add link speed configuration") Cc: stable@vger.kernel.org # 5.15.x Signed-off-by: Mohammad Athari Bin Ismail
mohammad.athari.ismail@intel.com
drivers/net/phy/marvell.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 4fcfca4e1702..2a73a959b48b 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -1932,6 +1932,12 @@ static void marvell_get_stats(struct phy_device
*phydev,
data[i] = marvell_get_stat(phydev, i); }+static int marvell_loopback(struct phy_device *phydev, bool enable) {
- return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK,
enable ? BMCR_LOOPBACK : 0);+}
static int marvell_vct5_wait_complete(struct phy_device *phydev) { int i; @@ -3078,7 +3084,7 @@ static struct phy_driver marvell_drivers[] = { .get_sset_count = marvell_get_sset_count, .get_strings = marvell_get_strings, .get_stats = marvell_get_stats,
.set_loopback = genphy_loopback,
.set_loopback = marvell_loopback,