[PATCH 0/3] More SFP/phylink fixes

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0/3] More SFP/phylink fixes
@ 2017-12-15 16:03 Russell King - ARM Linux
  2017-12-15 16:09 ` [PATCH 1/3] sfp: fix non-detection of PHY Russell King
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Russell King - ARM Linux @ 2017-12-15 16:03 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli; +Cc: netdev

Hi,

This series fixes a few more bits with sfp/phylink, particularly
confusion with the right way to test for the RTNL mutex being
held, a change in 2016 to the mdiobus_scan() behaviour that wasn't
noticed, and a fix for reading module EEPROMs.

 drivers/net/phy/phylink.c | 34 +++++++++++++++++-----------------
 drivers/net/phy/sfp.c     | 15 +++++++--------
 2 files changed, 24 insertions(+), 25 deletions(-)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] sfp: fix non-detection of PHY
  2017-12-15 16:03 [PATCH 0/3] More SFP/phylink fixes Russell King - ARM Linux
@ 2017-12-15 16:09 ` Russell King
  2017-12-15 16:09 ` [PATCH 2/3] sfp: fix EEPROM reading in the case of non-SFF8472 SFPs Russell King
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Russell King @ 2017-12-15 16:09 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli; +Cc: netdev

The detection of a PHY changed in commit e98a3aabf85f ("mdio_bus: don't
return NULL from mdiobus_scan()") which now causes sfp to print an
error message.  Update for this change.

Fixes: 73970055450e ("sfp: add SFP module support")
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/sfp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 96511557eb2c..1a958c7b912d 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -356,12 +356,12 @@ static void sfp_sm_probe_phy(struct sfp *sfp)
 	msleep(T_PHY_RESET_MS);
 
 	phy = mdiobus_scan(sfp->i2c_mii, SFP_PHY_ADDR);
-	if (IS_ERR(phy)) {
-		dev_err(sfp->dev, "mdiobus scan returned %ld\n", PTR_ERR(phy));
+	if (phy == ERR_PTR(-ENODEV)) {
+		dev_info(sfp->dev, "no PHY detected\n");
 		return;
 	}
-	if (!phy) {
-		dev_info(sfp->dev, "no PHY detected\n");
+	if (IS_ERR(phy)) {
+		dev_err(sfp->dev, "mdiobus scan returned %ld\n", PTR_ERR(phy));
 		return;
 	}
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] sfp: fix EEPROM reading in the case of non-SFF8472 SFPs
  2017-12-15 16:03 [PATCH 0/3] More SFP/phylink fixes Russell King - ARM Linux
  2017-12-15 16:09 ` [PATCH 1/3] sfp: fix non-detection of PHY Russell King
@ 2017-12-15 16:09 ` Russell King
  2017-12-15 16:09 ` [PATCH 3/3] phylink: fix locking asserts Russell King
  2017-12-18 19:58 ` [PATCH 0/3] More SFP/phylink fixes David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: Russell King @ 2017-12-15 16:09 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli; +Cc: netdev

The EEPROM reading was trying to read from the second EEPROM address
if we requested the last byte from the SFF8079 EEPROM, which caused a
failure when the second EEPROM is not present.  Discovered with a
S-RJ01 SFP module.  Fix this.

Fixes: 73970055450e ("sfp: add SFP module support")
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/sfp.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 1a958c7b912d..ee6b2e041171 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -724,20 +724,19 @@ static int sfp_module_eeprom(struct sfp *sfp, struct ethtool_eeprom *ee,
 		len = min_t(unsigned int, last, ETH_MODULE_SFF_8079_LEN);
 		len -= first;
 
-		ret = sfp->read(sfp, false, first, data, len);
+		ret = sfp_read(sfp, false, first, data, len);
 		if (ret < 0)
 			return ret;
 
 		first += len;
 		data += len;
 	}
-	if (first >= ETH_MODULE_SFF_8079_LEN &&
-	    first < ETH_MODULE_SFF_8472_LEN) {
+	if (first < ETH_MODULE_SFF_8472_LEN && last > ETH_MODULE_SFF_8079_LEN) {
 		len = min_t(unsigned int, last, ETH_MODULE_SFF_8472_LEN);
 		len -= first;
 		first -= ETH_MODULE_SFF_8079_LEN;
 
-		ret = sfp->read(sfp, true, first, data, len);
+		ret = sfp_read(sfp, true, first, data, len);
 		if (ret < 0)
 			return ret;
 	}
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] phylink: fix locking asserts
  2017-12-15 16:03 [PATCH 0/3] More SFP/phylink fixes Russell King - ARM Linux
  2017-12-15 16:09 ` [PATCH 1/3] sfp: fix non-detection of PHY Russell King
  2017-12-15 16:09 ` [PATCH 2/3] sfp: fix EEPROM reading in the case of non-SFF8472 SFPs Russell King
@ 2017-12-15 16:09 ` Russell King
  2017-12-18 19:58 ` [PATCH 0/3] More SFP/phylink fixes David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: Russell King @ 2017-12-15 16:09 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli; +Cc: netdev

Use ASSERT_RTNL() rather than WARN_ON(!lockdep_rtnl_is_held()) which
stops working when lockdep fires, and we end up with lots of warnings.

Fixes: 9525ae83959b ("phylink: add phylink infrastructure")
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index c89b8c63f16a..de1d65bc977c 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -804,7 +804,7 @@ void phylink_disconnect_phy(struct phylink *pl)
 {
 	struct phy_device *phy;
 
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	phy = pl->phydev;
 	if (phy) {
@@ -874,7 +874,7 @@ EXPORT_SYMBOL_GPL(phylink_mac_change);
  */
 void phylink_start(struct phylink *pl)
 {
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	netdev_info(pl->netdev, "configuring for %s/%s link mode\n",
 		    phylink_an_mode_str(pl->link_an_mode),
@@ -914,7 +914,7 @@ EXPORT_SYMBOL_GPL(phylink_start);
  */
 void phylink_stop(struct phylink *pl)
 {
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	if (pl->phydev)
 		phy_stop(pl->phydev);
@@ -938,7 +938,7 @@ EXPORT_SYMBOL_GPL(phylink_stop);
  */
 void phylink_ethtool_get_wol(struct phylink *pl, struct ethtool_wolinfo *wol)
 {
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	wol->supported = 0;
 	wol->wolopts = 0;
@@ -963,7 +963,7 @@ int phylink_ethtool_set_wol(struct phylink *pl, struct ethtool_wolinfo *wol)
 {
 	int ret = -EOPNOTSUPP;
 
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	if (pl->phydev)
 		ret = phy_ethtool_set_wol(pl->phydev, wol);
@@ -1008,7 +1008,7 @@ int phylink_ethtool_ksettings_get(struct phylink *pl,
 {
 	struct phylink_link_state link_state;
 
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	if (pl->phydev) {
 		phy_ethtool_ksettings_get(pl->phydev, kset);
@@ -1061,7 +1061,7 @@ int phylink_ethtool_ksettings_set(struct phylink *pl,
 	struct phylink_link_state config;
 	int ret;
 
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	if (kset->base.autoneg != AUTONEG_DISABLE &&
 	    kset->base.autoneg != AUTONEG_ENABLE)
@@ -1162,7 +1162,7 @@ int phylink_ethtool_nway_reset(struct phylink *pl)
 {
 	int ret = 0;
 
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	if (pl->phydev)
 		ret = phy_restart_aneg(pl->phydev);
@@ -1180,7 +1180,7 @@ EXPORT_SYMBOL_GPL(phylink_ethtool_nway_reset);
 void phylink_ethtool_get_pauseparam(struct phylink *pl,
 				    struct ethtool_pauseparam *pause)
 {
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	pause->autoneg = !!(pl->link_config.pause & MLO_PAUSE_AN);
 	pause->rx_pause = !!(pl->link_config.pause & MLO_PAUSE_RX);
@@ -1198,7 +1198,7 @@ int phylink_ethtool_set_pauseparam(struct phylink *pl,
 {
 	struct phylink_link_state *config = &pl->link_config;
 
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	if (!phylink_test(pl->supported, Pause) &&
 	    !phylink_test(pl->supported, Asym_Pause))
@@ -1284,7 +1284,7 @@ int phylink_get_eee_err(struct phylink *pl)
 {
 	int ret = 0;
 
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	if (pl->phydev)
 		ret = phy_get_eee_err(pl->phydev);
@@ -1302,7 +1302,7 @@ int phylink_ethtool_get_eee(struct phylink *pl, struct ethtool_eee *eee)
 {
 	int ret = -EOPNOTSUPP;
 
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	if (pl->phydev)
 		ret = phy_ethtool_get_eee(pl->phydev, eee);
@@ -1320,7 +1320,7 @@ int phylink_ethtool_set_eee(struct phylink *pl, struct ethtool_eee *eee)
 {
 	int ret = -EOPNOTSUPP;
 
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	if (pl->phydev)
 		ret = phy_ethtool_set_eee(pl->phydev, eee);
@@ -1510,7 +1510,7 @@ int phylink_mii_ioctl(struct phylink *pl, struct ifreq *ifr, int cmd)
 	struct mii_ioctl_data *mii = if_mii(ifr);
 	int  ret;
 
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	if (pl->phydev) {
 		/* PHYs only exist for MLO_AN_PHY and SGMII */
@@ -1578,7 +1578,7 @@ static int phylink_sfp_module_insert(void *upstream,
 	port = sfp_parse_port(pl->sfp_bus, id, support);
 	iface = sfp_parse_interface(pl->sfp_bus, id);
 
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	switch (iface) {
 	case PHY_INTERFACE_MODE_SGMII:
@@ -1647,7 +1647,7 @@ static void phylink_sfp_link_down(void *upstream)
 {
 	struct phylink *pl = upstream;
 
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	set_bit(PHYLINK_DISABLE_LINK, &pl->phylink_disable_state);
 	flush_work(&pl->resolve);
@@ -1659,7 +1659,7 @@ static void phylink_sfp_link_up(void *upstream)
 {
 	struct phylink *pl = upstream;
 
-	WARN_ON(!lockdep_rtnl_is_held());
+	ASSERT_RTNL();
 
 	clear_bit(PHYLINK_DISABLE_LINK, &pl->phylink_disable_state);
 	phylink_run_resolve(pl);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] More SFP/phylink fixes
  2017-12-15 16:03 [PATCH 0/3] More SFP/phylink fixes Russell King - ARM Linux
                   ` (2 preceding siblings ...)
  2017-12-15 16:09 ` [PATCH 3/3] phylink: fix locking asserts Russell King
@ 2017-12-18 19:58 ` David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2017-12-18 19:58 UTC (permalink / raw)
  To: linux; +Cc: andrew, f.fainelli, netdev

From: Russell King - ARM Linux <linux@armlinux.org.uk>
Date: Fri, 15 Dec 2017 16:03:44 +0000

> This series fixes a few more bits with sfp/phylink, particularly
> confusion with the right way to test for the RTNL mutex being
> held, a change in 2016 to the mdiobus_scan() behaviour that wasn't
> noticed, and a fix for reading module EEPROMs.

Series applied to net-next, because that's the tree this actually
applies cleanly to.

Please be explicit about which tree of mine you are targetting
in the future.

Thank you.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-12-18 19:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-15 16:03 [PATCH 0/3] More SFP/phylink fixes Russell King - ARM Linux
2017-12-15 16:09 ` [PATCH 1/3] sfp: fix non-detection of PHY Russell King
2017-12-15 16:09 ` [PATCH 2/3] sfp: fix EEPROM reading in the case of non-SFF8472 SFPs Russell King
2017-12-15 16:09 ` [PATCH 3/3] phylink: fix locking asserts Russell King
2017-12-18 19:58 ` [PATCH 0/3] More SFP/phylink fixes David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).