This patch to implement Marvell PHY specific loopback callback function. Verified working on Marvell 88E1510 at 1Gbps speed only. For 100Mbps and 10Mbps, found that the PHY loopback not able to function properly. Possible due to limitation in Marvell 88E1510 PHY.
Tested on Intel Elkhart Lake platform (Synopsys Designware QoS MAC and Marvell 88E1510 PHY).
Mohammad Athari Bin Ismail (1): net: phy: marvell: add Marvell specific PHY loopback
drivers/net/phy/marvell.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
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.
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, .get_tunable = m88e1011_get_tunable, .set_tunable = m88e1011_set_tunable, .cable_test_start = marvell_vct7_cable_test_start,
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.
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,
-----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 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 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"
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,
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 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 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.
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,
-----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,
On Mon, Jan 10, 2022 at 02:21:16PM +0800, Mohammad Athari Bin Ismail wrote:
This patch to implement Marvell PHY specific loopback callback function. Verified working on Marvell 88E1510 at 1Gbps speed only. For 100Mbps and 10Mbps, found that the PHY loopback not able to function properly. Possible due to limitation in Marvell 88E1510 PHY.
This is valuable information that should be in the commit message for the patch.
-----Original Message----- From: Russell King linux@armlinux.org.uk Sent: Monday, January 10, 2022 5:36 PM To: Ismail, Mohammad Athari mohammad.athari.ismail@intel.com Cc: Andrew Lunn andrew@lunn.ch; David S . Miller davem@davemloft.net; Jakub Kicinski kuba@kernel.org; Oleksij Rempel linux@rempel-privat.de; Heiner Kallweit hkallweit1@gmail.com; netdev@vger.kernel.org; linux- kernel@vger.kernel.org; stable@vger.kernel.org Subject: Re: [PATCH net 0/1] net: phy: marvell: add Marvell specific PHY loopback
On Mon, Jan 10, 2022 at 02:21:16PM +0800, Mohammad Athari Bin Ismail wrote:
This patch to implement Marvell PHY specific loopback callback function. Verified working on Marvell 88E1510 at 1Gbps speed only. For 100Mbps and 10Mbps, found that the PHY loopback not able to function properly. Possible due to limitation in Marvell 88E1510 PHY.
This is valuable information that should be in the commit message for the patch.
Sure. I will add this information in commit message for next version patch.
-Athari-
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
linux-stable-mirror@lists.linaro.org
-
Heiner Kallweit -
Ismail, Mohammad Athari -
Mohammad Athari Bin Ismail
-
Russell King (Oracle)