[PATCH NET 1/2] net: phy: Add phy loopback support in net phy framework

Subsystems: ethernet phy library, networking drivers, the rest · Maintainers: Andrew Lunn, Heiner Kallweit, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds

STALE3414d

3 messages, 2 authors, 2017-06-23 · open the first message on its own page

[PATCH NET 1/2] net: phy: Add phy loopback support in net phy framework

From: Lin Yun Sheng <hidden>
Date: 2017-06-22 08:23:18

This patch add set_loopback in phy_driver, which is used by Mac
driver to enable or disable a phy. it also add a generic
genphy_loopback function, which use BMCR loopback bit to enable
or disable a phy.

Signed-off-by: Lin Yun Sheng <redacted>
---
 drivers/net/phy/marvell.c    |  1 +
 drivers/net/phy/phy_device.c | 23 +++++++++++++++++++++++
 include/linux/phy.h          |  2 ++
 3 files changed, 26 insertions(+)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 57297ba..01a1586 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c

@@ -2094,6 +2094,7 @@ static int m88e1510_probe(struct phy_device *phydev)
 		.get_sset_count = marvell_get_sset_count,
 		.get_strings = marvell_get_strings,
 		.get_stats = marvell_get_stats,
+		.set_loopback = genphy_loopback,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1540,

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 1219eea..13b36fa 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c

@@ -1628,6 +1628,27 @@ static int gen10g_resume(struct phy_device *phydev)
 	return 0;
 }
 
+int genphy_loopback(struct phy_device *phydev, bool enable)
+{
+	int value;
+
+	if (enable) {
+		value = phy_read(phydev, MII_BMCR);
+		phy_write(phydev, MII_BMCR, value | BMCR_LOOPBACK);
+	} else {
+		value = phy_read(phydev, MII_BMCR);
+		phy_write(phydev, MII_BMCR, value & ~BMCR_LOOPBACK);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(genphy_loopback);
+
+static int gen10g_loopback(struct phy_device *phydev, bool enable)
+{
+	return 0;
+}
+
 static int __set_phy_supported(struct phy_device *phydev, u32 max_speed)
 {
 	/* The default values for phydev->supported are provided by the PHY

@@ -1874,6 +1895,7 @@ void phy_drivers_unregister(struct phy_driver *drv, int n)
 	.read_status	= genphy_read_status,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
+	.set_loopback	= genphy_loopback,
 }, {
 	.phy_id         = 0xffffffff,
 	.phy_id_mask    = 0xffffffff,

@@ -1885,6 +1907,7 @@ void phy_drivers_unregister(struct phy_driver *drv, int n)
 	.read_status    = gen10g_read_status,
 	.suspend        = gen10g_suspend,
 	.resume         = gen10g_resume,
+	.set_loopback	= gen10g_loopback,
 } };
 
 static int __init phy_init(void)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index e76e4ad..9362e22 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h

@@ -639,6 +639,7 @@ struct phy_driver {
 	int (*set_tunable)(struct phy_device *dev,
 			    struct ethtool_tunable *tuna,
 			    const void *data);
+	int (*set_loopback)(struct phy_device *dev, bool enable);
 };
 #define to_phy_driver(d) container_of(to_mdio_common_driver(d),		\
 				      struct phy_driver, mdiodrv)

@@ -825,6 +826,7 @@ void phy_attached_print(struct phy_device *phydev, const char *fmt, ...)
 int genphy_read_status(struct phy_device *phydev);
 int genphy_suspend(struct phy_device *phydev);
 int genphy_resume(struct phy_device *phydev);
+int genphy_loopback(struct phy_device *phydev, bool enable);
 int genphy_soft_reset(struct phy_device *phydev);
 static inline int genphy_no_soft_reset(struct phy_device *phydev)
 {

-- 
1.9.1

Re: [PATCH NET 1/2] net: phy: Add phy loopback support in net phy framework

From: Andrew Lunn <andrew@lunn.ch>
Date: 2017-06-23 03:16:46

quoted hunk

+int genphy_loopback(struct phy_device *phydev, bool enable)
+{
+	int value;
+
+	if (enable) {
+		value = phy_read(phydev, MII_BMCR);
+		phy_write(phydev, MII_BMCR, value | BMCR_LOOPBACK);
+	} else {
+		value = phy_read(phydev, MII_BMCR);
+		phy_write(phydev, MII_BMCR, value & ~BMCR_LOOPBACK);
+	}

You should add error checking here. Both phy_read and phy_write can
return an error. Also, do the read and write once.

quoted hunk

+
+	return 0;
+}
+EXPORT_SYMBOL(genphy_loopback);
+
+static int gen10g_loopback(struct phy_device *phydev, bool enable)
+{
+	return 0;
+}

This is pointless. The core code in phy.c should first check if the
function exists before calling it. So not having a 10g method is fine.

	 Andrew

Re: [PATCH NET 1/2] net: phy: Add phy loopback support in net phy framework

From: Yunsheng Lin <hidden>
Date: 2017-06-23 03:58:28

Hi, Andrew

On 2017/6/23 11:16, Andrew Lunn wrote:

quoted

+int genphy_loopback(struct phy_device *phydev, bool enable)
+{
+	int value;
+
+	if (enable) {
+		value = phy_read(phydev, MII_BMCR);
+		phy_write(phydev, MII_BMCR, value | BMCR_LOOPBACK);
+	} else {
+		value = phy_read(phydev, MII_BMCR);
+		phy_write(phydev, MII_BMCR, value & ~BMCR_LOOPBACK);
+	}

You should add error checking here. Both phy_read and phy_write can
return an error. Also, do the read and write once.

Thanks for pointing out, will do that next vesion.

quoted

+
+	return 0;
+}
+EXPORT_SYMBOL(genphy_loopback);
+
+static int gen10g_loopback(struct phy_device *phydev, bool enable)
+{
+	return 0;
+}

This is pointless. The core code in phy.c should first check if the
function exists before calling it. So not having a 10g method is fine.

will remove it next vesion.

Best Regards
Yunsheng Lin

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help