netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/5] DSA cleanup and fixes
@ 2016-03-11 23:01 Andrew Lunn
  2016-03-11 23:01 ` [PATCH net-next 1/5] dsa: Rename mv88e6123_61_65 to mv88e6123 to be consistent Andrew Lunn
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Andrew Lunn @ 2016-03-11 23:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Vivien Didelot, Andrew Lunn

The RFC patchset for re-architecturing DSA probing contains a few
standalone patches, either clean up or fixes. This pulls them out for
submission.

Andrew Lunn (5):
  dsa: Rename mv88e6123_61_65 to mv88e6123 to be consistent
  dsa: slave: Don't reference NULL pointer during phy_disconnect
  dsa: Destroy fixed link phys after the phy has been disconnected
  dsa: dsa: Fix freeing of fixed-phys from user ports.
  phy: fixed: Fix removal of phys.

 arch/arm/configs/multi_v5_defconfig |   2 +-
 arch/arm/configs/mvebu_v5_defconfig |   2 +-
 arch/arm/configs/orion5x_defconfig  |   2 +-
 arch/tile/configs/tilegx_defconfig  |   2 +-
 arch/tile/configs/tilepro_defconfig |   2 +-
 drivers/net/dsa/Kconfig             |   2 +-
 drivers/net/dsa/Makefile            |   4 +-
 drivers/net/dsa/mv88e6123.c         | 124 ++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6123_61_65.c   | 124 ------------------------------------
 drivers/net/dsa/mv88e6xxx.c         |   8 +--
 drivers/net/dsa/mv88e6xxx.h         |   2 +-
 drivers/net/phy/fixed_phy.c         |  11 +++-
 include/linux/phy_fixed.h           |   5 +-
 net/dsa/dsa.c                       |  27 ++++----
 net/dsa/slave.c                     |  12 ++--
 15 files changed, 167 insertions(+), 162 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6123.c
 delete mode 100644 drivers/net/dsa/mv88e6123_61_65.c

-- 
2.7.0

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

* [PATCH net-next 1/5] dsa: Rename mv88e6123_61_65 to mv88e6123 to be consistent
  2016-03-11 23:01 [PATCH net-next 0/5] DSA cleanup and fixes Andrew Lunn
@ 2016-03-11 23:01 ` Andrew Lunn
  2016-03-13  7:10   ` Vivien Didelot
  2016-03-11 23:01 ` [PATCH net-next 2/5] dsa: slave: Don't reference NULL pointer during phy_disconnect Andrew Lunn
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Andrew Lunn @ 2016-03-11 23:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Vivien Didelot, Andrew Lunn

All the drivers support multiple chips, but mv88e6123_61_65 is the
only one that reflects this in its naming. Change it to be consistent
with the other drivers.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 arch/arm/configs/multi_v5_defconfig |   2 +-
 arch/arm/configs/mvebu_v5_defconfig |   2 +-
 arch/arm/configs/orion5x_defconfig  |   2 +-
 arch/tile/configs/tilegx_defconfig  |   2 +-
 arch/tile/configs/tilepro_defconfig |   2 +-
 drivers/net/dsa/Kconfig             |   2 +-
 drivers/net/dsa/Makefile            |   4 +-
 drivers/net/dsa/mv88e6123.c         | 124 ++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6123_61_65.c   | 124 ------------------------------------
 drivers/net/dsa/mv88e6xxx.c         |   8 +--
 drivers/net/dsa/mv88e6xxx.h         |   2 +-
 11 files changed, 137 insertions(+), 137 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6123.c
 delete mode 100644 drivers/net/dsa/mv88e6123_61_65.c

diff --git a/arch/arm/configs/multi_v5_defconfig b/arch/arm/configs/multi_v5_defconfig
index 1f9ca4737ef6..afb1f62fb05e 100644
--- a/arch/arm/configs/multi_v5_defconfig
+++ b/arch/arm/configs/multi_v5_defconfig
@@ -91,7 +91,7 @@ CONFIG_SATA_MV=y
 CONFIG_NETDEVICES=y
 CONFIG_NET_DSA_MV88E6060=y
 CONFIG_NET_DSA_MV88E6131=y
-CONFIG_NET_DSA_MV88E6123_61_65=y
+CONFIG_NET_DSA_MV88E6123=y
 CONFIG_NET_DSA_MV88E6171=y
 CONFIG_NET_DSA_MV88E6352=y
 CONFIG_MV643XX_ETH=y
diff --git a/arch/arm/configs/mvebu_v5_defconfig b/arch/arm/configs/mvebu_v5_defconfig
index af29780accdc..6c4c54037bc4 100644
--- a/arch/arm/configs/mvebu_v5_defconfig
+++ b/arch/arm/configs/mvebu_v5_defconfig
@@ -92,7 +92,7 @@ CONFIG_SATA_MV=y
 CONFIG_NETDEVICES=y
 CONFIG_NET_DSA_MV88E6060=y
 CONFIG_NET_DSA_MV88E6131=y
-CONFIG_NET_DSA_MV88E6123_61_65=y
+CONFIG_NET_DSA_MV88E6123=y
 CONFIG_NET_DSA_MV88E6171=y
 CONFIG_NET_DSA_MV88E6352=y
 CONFIG_MV643XX_ETH=y
diff --git a/arch/arm/configs/orion5x_defconfig b/arch/arm/configs/orion5x_defconfig
index 5876ce7af130..6a5bc27538f1 100644
--- a/arch/arm/configs/orion5x_defconfig
+++ b/arch/arm/configs/orion5x_defconfig
@@ -86,7 +86,7 @@ CONFIG_SATA_MV=y
 CONFIG_NETDEVICES=y
 CONFIG_MII=y
 CONFIG_NET_DSA_MV88E6131=y
-CONFIG_NET_DSA_MV88E6123_61_65=y
+CONFIG_NET_DSA_MV88E6123=y
 CONFIG_MV643XX_ETH=y
 CONFIG_MARVELL_PHY=y
 # CONFIG_INPUT_MOUSEDEV is not set
diff --git a/arch/tile/configs/tilegx_defconfig b/arch/tile/configs/tilegx_defconfig
index 37dc9364c4a1..984fa00a8c25 100644
--- a/arch/tile/configs/tilegx_defconfig
+++ b/arch/tile/configs/tilegx_defconfig
@@ -222,7 +222,7 @@ CONFIG_TUN=y
 CONFIG_VETH=m
 CONFIG_NET_DSA_MV88E6060=y
 CONFIG_NET_DSA_MV88E6131=y
-CONFIG_NET_DSA_MV88E6123_61_65=y
+CONFIG_NET_DSA_MV88E6123=y
 CONFIG_SKY2=y
 CONFIG_PTP_1588_CLOCK_TILEGX=y
 # CONFIG_WLAN is not set
diff --git a/arch/tile/configs/tilepro_defconfig b/arch/tile/configs/tilepro_defconfig
index 76a2781dec2c..71ad9f7e40c9 100644
--- a/arch/tile/configs/tilepro_defconfig
+++ b/arch/tile/configs/tilepro_defconfig
@@ -341,7 +341,7 @@ CONFIG_TUN=y
 CONFIG_VETH=m
 CONFIG_NET_DSA_MV88E6060=y
 CONFIG_NET_DSA_MV88E6131=y
-CONFIG_NET_DSA_MV88E6123_61_65=y
+CONFIG_NET_DSA_MV88E6123=y
 # CONFIG_NET_VENDOR_3COM is not set
 CONFIG_E1000E=y
 # CONFIG_WLAN is not set
diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 4c483d937481..90ba003d8fdf 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -27,7 +27,7 @@ config NET_DSA_MV88E6131
 	  This enables support for the Marvell 88E6085/6095/6095F/6131
 	  ethernet switch chips.
 
-config NET_DSA_MV88E6123_61_65
+config NET_DSA_MV88E6123
 	tristate "Marvell 88E6123/6161/6165 ethernet switch chip support"
 	depends on NET_DSA
 	select NET_DSA_MV88E6XXX
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index e2d51c4b9382..a6e09939be65 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -1,8 +1,8 @@
 obj-$(CONFIG_NET_DSA_MV88E6060) += mv88e6060.o
 obj-$(CONFIG_NET_DSA_MV88E6XXX) += mv88e6xxx_drv.o
 mv88e6xxx_drv-y += mv88e6xxx.o
-ifdef CONFIG_NET_DSA_MV88E6123_61_65
-mv88e6xxx_drv-y += mv88e6123_61_65.o
+ifdef CONFIG_NET_DSA_MV88E6123
+mv88e6xxx_drv-y += mv88e6123.o
 endif
 ifdef CONFIG_NET_DSA_MV88E6131
 mv88e6xxx_drv-y += mv88e6131.o
diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
new file mode 100644
index 000000000000..69a6f79dcb10
--- /dev/null
+++ b/drivers/net/dsa/mv88e6123.c
@@ -0,0 +1,124 @@
+/*
+ * net/dsa/mv88e6123_61_65.c - Marvell 88e6123/6161/6165 switch chip support
+ * Copyright (c) 2008-2009 Marvell Semiconductor
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/delay.h>
+#include <linux/jiffies.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/phy.h>
+#include <net/dsa.h>
+#include "mv88e6xxx.h"
+
+static const struct mv88e6xxx_switch_id mv88e6123_table[] = {
+	{ PORT_SWITCH_ID_6123, "Marvell 88E6123" },
+	{ PORT_SWITCH_ID_6123_A1, "Marvell 88E6123 (A1)" },
+	{ PORT_SWITCH_ID_6123_A2, "Marvell 88E6123 (A2)" },
+	{ PORT_SWITCH_ID_6161, "Marvell 88E6161" },
+	{ PORT_SWITCH_ID_6161_A1, "Marvell 88E6161 (A1)" },
+	{ PORT_SWITCH_ID_6161_A2, "Marvell 88E6161 (A2)" },
+	{ PORT_SWITCH_ID_6165, "Marvell 88E6165" },
+	{ PORT_SWITCH_ID_6165_A1, "Marvell 88E6165 (A1)" },
+	{ PORT_SWITCH_ID_6165_A2, "Marvell 88e6165 (A2)" },
+};
+
+static char *mv88e6123_probe(struct device *host_dev, int sw_addr)
+{
+	return mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6123_table,
+				     ARRAY_SIZE(mv88e6123_table));
+}
+
+static int mv88e6123_setup_global(struct dsa_switch *ds)
+{
+	u32 upstream_port = dsa_upstream_port(ds);
+	int ret;
+	u32 reg;
+
+	ret = mv88e6xxx_setup_global(ds);
+	if (ret)
+		return ret;
+
+	/* Disable the PHY polling unit (since there won't be any
+	 * external PHYs to poll), don't discard packets with
+	 * excessive collisions, and mask all interrupt sources.
+	 */
+	REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL, 0x0000);
+
+	/* Configure the upstream port, and configure the upstream
+	 * port as the port to which ingress and egress monitor frames
+	 * are to be sent.
+	 */
+	reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
+		upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
+		upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT;
+	REG_WRITE(REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
+
+	/* Disable remote management for now, and set the switch's
+	 * DSA device number.
+	 */
+	REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL_2, ds->index & 0x1f);
+
+	return 0;
+}
+
+static int mv88e6123_setup(struct dsa_switch *ds)
+{
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	int ret;
+
+	ret = mv88e6xxx_setup_common(ds);
+	if (ret < 0)
+		return ret;
+
+	switch (ps->id) {
+	case PORT_SWITCH_ID_6123:
+		ps->num_ports = 3;
+		break;
+	case PORT_SWITCH_ID_6161:
+	case PORT_SWITCH_ID_6165:
+		ps->num_ports = 6;
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	ret = mv88e6xxx_switch_reset(ds, false);
+	if (ret < 0)
+		return ret;
+
+	ret = mv88e6123_setup_global(ds);
+	if (ret < 0)
+		return ret;
+
+	return mv88e6xxx_setup_ports(ds);
+}
+
+struct dsa_switch_driver mv88e6123_switch_driver = {
+	.tag_protocol		= DSA_TAG_PROTO_EDSA,
+	.priv_size		= sizeof(struct mv88e6xxx_priv_state),
+	.probe			= mv88e6123_probe,
+	.setup			= mv88e6123_setup,
+	.set_addr		= mv88e6xxx_set_addr_indirect,
+	.phy_read		= mv88e6xxx_phy_read,
+	.phy_write		= mv88e6xxx_phy_write,
+	.get_strings		= mv88e6xxx_get_strings,
+	.get_ethtool_stats	= mv88e6xxx_get_ethtool_stats,
+	.get_sset_count		= mv88e6xxx_get_sset_count,
+	.adjust_link		= mv88e6xxx_adjust_link,
+#ifdef CONFIG_NET_DSA_HWMON
+	.get_temp		= mv88e6xxx_get_temp,
+#endif
+	.get_regs_len		= mv88e6xxx_get_regs_len,
+	.get_regs		= mv88e6xxx_get_regs,
+};
+
+MODULE_ALIAS("platform:mv88e6123");
+MODULE_ALIAS("platform:mv88e6161");
+MODULE_ALIAS("platform:mv88e6165");
diff --git a/drivers/net/dsa/mv88e6123_61_65.c b/drivers/net/dsa/mv88e6123_61_65.c
deleted file mode 100644
index d4fcf4570d95..000000000000
--- a/drivers/net/dsa/mv88e6123_61_65.c
+++ /dev/null
@@ -1,124 +0,0 @@
-/*
- * net/dsa/mv88e6123_61_65.c - Marvell 88e6123/6161/6165 switch chip support
- * Copyright (c) 2008-2009 Marvell Semiconductor
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#include <linux/delay.h>
-#include <linux/jiffies.h>
-#include <linux/list.h>
-#include <linux/module.h>
-#include <linux/netdevice.h>
-#include <linux/phy.h>
-#include <net/dsa.h>
-#include "mv88e6xxx.h"
-
-static const struct mv88e6xxx_switch_id mv88e6123_61_65_table[] = {
-	{ PORT_SWITCH_ID_6123, "Marvell 88E6123" },
-	{ PORT_SWITCH_ID_6123_A1, "Marvell 88E6123 (A1)" },
-	{ PORT_SWITCH_ID_6123_A2, "Marvell 88E6123 (A2)" },
-	{ PORT_SWITCH_ID_6161, "Marvell 88E6161" },
-	{ PORT_SWITCH_ID_6161_A1, "Marvell 88E6161 (A1)" },
-	{ PORT_SWITCH_ID_6161_A2, "Marvell 88E6161 (A2)" },
-	{ PORT_SWITCH_ID_6165, "Marvell 88E6165" },
-	{ PORT_SWITCH_ID_6165_A1, "Marvell 88E6165 (A1)" },
-	{ PORT_SWITCH_ID_6165_A2, "Marvell 88e6165 (A2)" },
-};
-
-static char *mv88e6123_61_65_probe(struct device *host_dev, int sw_addr)
-{
-	return mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6123_61_65_table,
-				     ARRAY_SIZE(mv88e6123_61_65_table));
-}
-
-static int mv88e6123_61_65_setup_global(struct dsa_switch *ds)
-{
-	u32 upstream_port = dsa_upstream_port(ds);
-	int ret;
-	u32 reg;
-
-	ret = mv88e6xxx_setup_global(ds);
-	if (ret)
-		return ret;
-
-	/* Disable the PHY polling unit (since there won't be any
-	 * external PHYs to poll), don't discard packets with
-	 * excessive collisions, and mask all interrupt sources.
-	 */
-	REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL, 0x0000);
-
-	/* Configure the upstream port, and configure the upstream
-	 * port as the port to which ingress and egress monitor frames
-	 * are to be sent.
-	 */
-	reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
-		upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
-		upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT;
-	REG_WRITE(REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
-
-	/* Disable remote management for now, and set the switch's
-	 * DSA device number.
-	 */
-	REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL_2, ds->index & 0x1f);
-
-	return 0;
-}
-
-static int mv88e6123_61_65_setup(struct dsa_switch *ds)
-{
-	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
-	int ret;
-
-	ret = mv88e6xxx_setup_common(ds);
-	if (ret < 0)
-		return ret;
-
-	switch (ps->id) {
-	case PORT_SWITCH_ID_6123:
-		ps->num_ports = 3;
-		break;
-	case PORT_SWITCH_ID_6161:
-	case PORT_SWITCH_ID_6165:
-		ps->num_ports = 6;
-		break;
-	default:
-		return -ENODEV;
-	}
-
-	ret = mv88e6xxx_switch_reset(ds, false);
-	if (ret < 0)
-		return ret;
-
-	ret = mv88e6123_61_65_setup_global(ds);
-	if (ret < 0)
-		return ret;
-
-	return mv88e6xxx_setup_ports(ds);
-}
-
-struct dsa_switch_driver mv88e6123_61_65_switch_driver = {
-	.tag_protocol		= DSA_TAG_PROTO_EDSA,
-	.priv_size		= sizeof(struct mv88e6xxx_priv_state),
-	.probe			= mv88e6123_61_65_probe,
-	.setup			= mv88e6123_61_65_setup,
-	.set_addr		= mv88e6xxx_set_addr_indirect,
-	.phy_read		= mv88e6xxx_phy_read,
-	.phy_write		= mv88e6xxx_phy_write,
-	.get_strings		= mv88e6xxx_get_strings,
-	.get_ethtool_stats	= mv88e6xxx_get_ethtool_stats,
-	.get_sset_count		= mv88e6xxx_get_sset_count,
-	.adjust_link		= mv88e6xxx_adjust_link,
-#ifdef CONFIG_NET_DSA_HWMON
-	.get_temp		= mv88e6xxx_get_temp,
-#endif
-	.get_regs_len		= mv88e6xxx_get_regs_len,
-	.get_regs		= mv88e6xxx_get_regs,
-};
-
-MODULE_ALIAS("platform:mv88e6123");
-MODULE_ALIAS("platform:mv88e6161");
-MODULE_ALIAS("platform:mv88e6165");
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 5f07524083c3..5309c738ff00 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -2989,8 +2989,8 @@ static int __init mv88e6xxx_init(void)
 #if IS_ENABLED(CONFIG_NET_DSA_MV88E6131)
 	register_switch_driver(&mv88e6131_switch_driver);
 #endif
-#if IS_ENABLED(CONFIG_NET_DSA_MV88E6123_61_65)
-	register_switch_driver(&mv88e6123_61_65_switch_driver);
+#if IS_ENABLED(CONFIG_NET_DSA_MV88E6123)
+	register_switch_driver(&mv88e6123_switch_driver);
 #endif
 #if IS_ENABLED(CONFIG_NET_DSA_MV88E6352)
 	register_switch_driver(&mv88e6352_switch_driver);
@@ -3010,8 +3010,8 @@ static void __exit mv88e6xxx_cleanup(void)
 #if IS_ENABLED(CONFIG_NET_DSA_MV88E6352)
 	unregister_switch_driver(&mv88e6352_switch_driver);
 #endif
-#if IS_ENABLED(CONFIG_NET_DSA_MV88E6123_61_65)
-	unregister_switch_driver(&mv88e6123_61_65_switch_driver);
+#if IS_ENABLED(CONFIG_NET_DSA_MV88E6123)
+	unregister_switch_driver(&mv88e6123_switch_driver);
 #endif
 #if IS_ENABLED(CONFIG_NET_DSA_MV88E6131)
 	unregister_switch_driver(&mv88e6131_switch_driver);
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 3425616987ed..281cefe86afd 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -519,7 +519,7 @@ int mv88e6xxx_phy_page_write(struct dsa_switch *ds, int port, int page,
 			     int reg, int val);
 
 extern struct dsa_switch_driver mv88e6131_switch_driver;
-extern struct dsa_switch_driver mv88e6123_61_65_switch_driver;
+extern struct dsa_switch_driver mv88e6123_switch_driver;
 extern struct dsa_switch_driver mv88e6352_switch_driver;
 extern struct dsa_switch_driver mv88e6171_switch_driver;
 
-- 
2.7.0

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

* [PATCH net-next 2/5] dsa: slave: Don't reference NULL pointer during phy_disconnect
  2016-03-11 23:01 [PATCH net-next 0/5] DSA cleanup and fixes Andrew Lunn
  2016-03-11 23:01 ` [PATCH net-next 1/5] dsa: Rename mv88e6123_61_65 to mv88e6123 to be consistent Andrew Lunn
@ 2016-03-11 23:01 ` Andrew Lunn
  2016-03-11 23:01 ` [PATCH net-next 3/5] dsa: Destroy fixed link phys after the phy has been disconnected Andrew Lunn
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Andrew Lunn @ 2016-03-11 23:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Vivien Didelot, Andrew Lunn

When the phy is disconnected, the parent pointer to the netdev it was
attached to is set to NULL. The code then tries to suspend the phy,
but dsa_slave_fixed_link_update needs the parent pointer to determine
which switch the phy is connected to. So it dereferenced a NULL
pointer. Check for this condition.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 net/dsa/slave.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 27bf03d11670..49056d90b179 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -896,11 +896,15 @@ static void dsa_slave_adjust_link(struct net_device *dev)
 static int dsa_slave_fixed_link_update(struct net_device *dev,
 				       struct fixed_phy_status *status)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->parent;
+	struct dsa_slave_priv *p;
+	struct dsa_switch *ds;
 
-	if (ds->drv->fixed_link_update)
-		ds->drv->fixed_link_update(ds, p->port, status);
+	if (dev) {
+		p = netdev_priv(dev);
+		ds = p->parent;
+		if (ds->drv->fixed_link_update)
+			ds->drv->fixed_link_update(ds, p->port, status);
+	}
 
 	return 0;
 }
-- 
2.7.0

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

* [PATCH net-next 3/5] dsa: Destroy fixed link phys after the phy has been disconnected
  2016-03-11 23:01 [PATCH net-next 0/5] DSA cleanup and fixes Andrew Lunn
  2016-03-11 23:01 ` [PATCH net-next 1/5] dsa: Rename mv88e6123_61_65 to mv88e6123 to be consistent Andrew Lunn
  2016-03-11 23:01 ` [PATCH net-next 2/5] dsa: slave: Don't reference NULL pointer during phy_disconnect Andrew Lunn
@ 2016-03-11 23:01 ` Andrew Lunn
  2016-03-11 23:01 ` [PATCH net-next 4/5] dsa: dsa: Fix freeing of fixed-phys from user ports Andrew Lunn
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Andrew Lunn @ 2016-03-11 23:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Vivien Didelot, Andrew Lunn

The phy is disconnected from the slave in dsa_slave_destroy(). Don't
destroy fixed link phys until after this, since there can be fixed
linked phys connected to ports.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 net/dsa/dsa.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index d8fb47fcad05..1018e7dcfcc9 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -430,7 +430,18 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
 		hwmon_device_unregister(ds->hwmon_dev);
 #endif
 
-	/* Disable configuration of the CPU and DSA ports */
+	/* Destroy network devices for physical switch ports. */
+	for (port = 0; port < DSA_MAX_PORTS; port++) {
+		if (!(ds->phys_port_mask & (1 << port)))
+			continue;
+
+		if (!ds->ports[port])
+			continue;
+
+		dsa_slave_destroy(ds->ports[port]);
+	}
+
+	/* Remove any fixed link PHYs */
 	for (port = 0; port < DSA_MAX_PORTS; port++) {
 		if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
 			continue;
@@ -448,17 +459,6 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
 		}
 	}
 
-	/* Destroy network devices for physical switch ports. */
-	for (port = 0; port < DSA_MAX_PORTS; port++) {
-		if (!(ds->phys_port_mask & (1 << port)))
-			continue;
-
-		if (!ds->ports[port])
-			continue;
-
-		dsa_slave_destroy(ds->ports[port]);
-	}
-
 	mdiobus_unregister(ds->slave_mii_bus);
 }
 
-- 
2.7.0

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

* [PATCH net-next 4/5] dsa: dsa: Fix freeing of fixed-phys from user ports.
  2016-03-11 23:01 [PATCH net-next 0/5] DSA cleanup and fixes Andrew Lunn
                   ` (2 preceding siblings ...)
  2016-03-11 23:01 ` [PATCH net-next 3/5] dsa: Destroy fixed link phys after the phy has been disconnected Andrew Lunn
@ 2016-03-11 23:01 ` Andrew Lunn
  2016-03-11 23:01 ` [PATCH net-next 5/5] phy: fixed: Fix removal of phys Andrew Lunn
  2016-03-14 19:44 ` [PATCH net-next 0/5] DSA cleanup and fixes David Miller
  5 siblings, 0 replies; 12+ messages in thread
From: Andrew Lunn @ 2016-03-11 23:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Vivien Didelot, Andrew Lunn

All ports types can have a fixed PHY associated with it. Remove the
check which limits removal to only CPU and DSA ports.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 net/dsa/dsa.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 1018e7dcfcc9..f100f340d93f 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -443,9 +443,6 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
 
 	/* Remove any fixed link PHYs */
 	for (port = 0; port < DSA_MAX_PORTS; port++) {
-		if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
-			continue;
-
 		port_dn = cd->port_dn[port];
 		if (of_phy_is_fixed_link(port_dn)) {
 			phydev = of_phy_find_device(port_dn);
-- 
2.7.0

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

* [PATCH net-next 5/5] phy: fixed: Fix removal of phys.
  2016-03-11 23:01 [PATCH net-next 0/5] DSA cleanup and fixes Andrew Lunn
                   ` (3 preceding siblings ...)
  2016-03-11 23:01 ` [PATCH net-next 4/5] dsa: dsa: Fix freeing of fixed-phys from user ports Andrew Lunn
@ 2016-03-11 23:01 ` Andrew Lunn
  2016-03-11 23:18   ` Florian Fainelli
  2016-03-14 19:44 ` [PATCH net-next 0/5] DSA cleanup and fixes David Miller
  5 siblings, 1 reply; 12+ messages in thread
From: Andrew Lunn @ 2016-03-11 23:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Vivien Didelot, Andrew Lunn

The fixed phys delete function simply removed the fixed phy from the
internal linked list and freed the memory. It however did not
unregister the associated phy device. This meant it was still possible
to find the phy device on the mdio bus.

Make fixed_phy_del() an internal function and add a
fixed_phy_unregister() to unregisters the phy device and then uses
fixed_phy_del() to free resources.

Modify DSA to use this new API function, so we don't leak phys.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/fixed_phy.c | 11 +++++++++--
 include/linux/phy_fixed.h   |  5 ++---
 net/dsa/dsa.c               |  4 +---
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index ab9c473d75ea..fc07a8866020 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -285,7 +285,7 @@ err_regs:
 }
 EXPORT_SYMBOL_GPL(fixed_phy_add);
 
-void fixed_phy_del(int phy_addr)
+static void fixed_phy_del(int phy_addr)
 {
 	struct fixed_mdio_bus *fmb = &platform_fmb;
 	struct fixed_phy *fp, *tmp;
@@ -300,7 +300,6 @@ void fixed_phy_del(int phy_addr)
 		}
 	}
 }
-EXPORT_SYMBOL_GPL(fixed_phy_del);
 
 static int phy_fixed_addr;
 static DEFINE_SPINLOCK(phy_fixed_addr_lock);
@@ -371,6 +370,14 @@ struct phy_device *fixed_phy_register(unsigned int irq,
 }
 EXPORT_SYMBOL_GPL(fixed_phy_register);
 
+void fixed_phy_unregister(struct phy_device *phy)
+{
+	phy_device_remove(phy);
+
+	fixed_phy_del(phy->mdio.addr);
+}
+EXPORT_SYMBOL_GPL(fixed_phy_unregister);
+
 static int __init fixed_mdio_bus_init(void)
 {
 	struct fixed_mdio_bus *fmb = &platform_fmb;
diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
index 2400d2ea4f34..1d41ec44e39d 100644
--- a/include/linux/phy_fixed.h
+++ b/include/linux/phy_fixed.h
@@ -19,7 +19,7 @@ extern struct phy_device *fixed_phy_register(unsigned int irq,
 					     struct fixed_phy_status *status,
 					     int link_gpio,
 					     struct device_node *np);
-extern void fixed_phy_del(int phy_addr);
+extern void fixed_phy_unregister(struct phy_device *phydev);
 extern int fixed_phy_set_link_update(struct phy_device *phydev,
 			int (*link_update)(struct net_device *,
 					   struct fixed_phy_status *));
@@ -40,9 +40,8 @@ static inline struct phy_device *fixed_phy_register(unsigned int irq,
 {
 	return ERR_PTR(-ENODEV);
 }
-static inline int fixed_phy_del(int phy_addr)
+static inline void fixed_phy_unregister(struct phy_device *phydev)
 {
-	return -ENODEV;
 }
 static inline int fixed_phy_set_link_update(struct phy_device *phydev,
 			int (*link_update)(struct net_device *,
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index f100f340d93f..c28c47463b7e 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -447,11 +447,9 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
 		if (of_phy_is_fixed_link(port_dn)) {
 			phydev = of_phy_find_device(port_dn);
 			if (phydev) {
-				int addr = phydev->mdio.addr;
-
 				phy_device_free(phydev);
 				of_node_put(port_dn);
-				fixed_phy_del(addr);
+				fixed_phy_unregister(phydev);
 			}
 		}
 	}
-- 
2.7.0

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

* Re: [PATCH net-next 5/5] phy: fixed: Fix removal of phys.
  2016-03-11 23:01 ` [PATCH net-next 5/5] phy: fixed: Fix removal of phys Andrew Lunn
@ 2016-03-11 23:18   ` Florian Fainelli
  2016-03-12 17:24     ` Andrew Lunn
  0 siblings, 1 reply; 12+ messages in thread
From: Florian Fainelli @ 2016-03-11 23:18 UTC (permalink / raw)
  To: Andrew Lunn, David Miller; +Cc: netdev, Vivien Didelot

On 11/03/16 15:01, Andrew Lunn wrote:
> The fixed phys delete function simply removed the fixed phy from the
> internal linked list and freed the memory. It however did not
> unregister the associated phy device. This meant it was still possible
> to find the phy device on the mdio bus.
> 
> Make fixed_phy_del() an internal function and add a
> fixed_phy_unregister() to unregisters the phy device and then uses
> fixed_phy_del() to free resources.
> 
> Modify DSA to use this new API function, so we don't leak phys.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/phy/fixed_phy.c | 11 +++++++++--
>  include/linux/phy_fixed.h   |  5 ++---
>  net/dsa/dsa.c               |  4 +---
>  3 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
> index ab9c473d75ea..fc07a8866020 100644
> --- a/drivers/net/phy/fixed_phy.c
> +++ b/drivers/net/phy/fixed_phy.c
> @@ -285,7 +285,7 @@ err_regs:
>  }
>  EXPORT_SYMBOL_GPL(fixed_phy_add);
>  
> -void fixed_phy_del(int phy_addr)
> +static void fixed_phy_del(int phy_addr)
>  {
>  	struct fixed_mdio_bus *fmb = &platform_fmb;
>  	struct fixed_phy *fp, *tmp;
> @@ -300,7 +300,6 @@ void fixed_phy_del(int phy_addr)
>  		}
>  	}
>  }
> -EXPORT_SYMBOL_GPL(fixed_phy_del);
>  
>  static int phy_fixed_addr;
>  static DEFINE_SPINLOCK(phy_fixed_addr_lock);
> @@ -371,6 +370,14 @@ struct phy_device *fixed_phy_register(unsigned int irq,
>  }
>  EXPORT_SYMBOL_GPL(fixed_phy_register);
>  
> +void fixed_phy_unregister(struct phy_device *phy)
> +{
> +	phy_device_remove(phy);
> +
> +	fixed_phy_del(phy->mdio.addr);

fixed_phy_del() should also make sure that there is no dangling
link_update callback registered, even though this is not fatal, as it
checks whether the phydev is NULL, we should automatically unregister
one by doing something like: fixed_phy_set_link_update(phydev, NULL) for
robustness.

LGTM to me otherwise, thanks!
-- 
Florian

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

* Re: [PATCH net-next 5/5] phy: fixed: Fix removal of phys.
  2016-03-11 23:18   ` Florian Fainelli
@ 2016-03-12 17:24     ` Andrew Lunn
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Lunn @ 2016-03-12 17:24 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David Miller, netdev, Vivien Didelot

> > +void fixed_phy_unregister(struct phy_device *phy)
> > +{
> > +	phy_device_remove(phy);
> > +
> > +	fixed_phy_del(phy->mdio.addr);
> 
> fixed_phy_del() should also make sure that there is no dangling
> link_update callback registered, even though this is not fatal, as it
> checks whether the phydev is NULL, we should automatically unregister
> one by doing something like: fixed_phy_set_link_update(phydev, NULL) for
> robustness.

Hi Florian

I don't see how it could happen. The link_update callback is only
called from fixed_mdio_read(). But we have just called
phy_device_remove(). There should not be anything using
fixed_mdio_read() after that. Also, fixed_phy_del() first removes fp
from the list of fmb->phys, meaning it is no longer possible to find
it, and then kfree(fp). So if there is some way we have a dangling
link_update, we are in big trouble, and the locking is badly broken...

    Andrew

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

* Re: [PATCH net-next 1/5] dsa: Rename mv88e6123_61_65 to mv88e6123 to be consistent
  2016-03-11 23:01 ` [PATCH net-next 1/5] dsa: Rename mv88e6123_61_65 to mv88e6123 to be consistent Andrew Lunn
@ 2016-03-13  7:10   ` Vivien Didelot
  2016-03-13 15:51     ` Andrew Lunn
  0 siblings, 1 reply; 12+ messages in thread
From: Vivien Didelot @ 2016-03-13  7:10 UTC (permalink / raw)
  To: Andrew Lunn, David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

Andrew Lunn <andrew@lunn.ch> writes:

> All the drivers support multiple chips, but mv88e6123_61_65 is the
> only one that reflects this in its naming. Change it to be consistent
> with the other drivers.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

When mv88e6xxx will become a driver by its own supporting different
devices, it'll be good to rename it to a reference driver as well, say
mv88e6352.

[...]

> diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
> new file mode 100644
> index 000000000000..69a6f79dcb10
> --- /dev/null
> +++ b/drivers/net/dsa/mv88e6123.c
> @@ -0,0 +1,124 @@

Note that to avoid the big diff above, you can use the -M option of
git-format-patch to detect file renames.

Thanks,
Vivien

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

* Re: [PATCH net-next 1/5] dsa: Rename mv88e6123_61_65 to mv88e6123 to be consistent
  2016-03-13  7:10   ` Vivien Didelot
@ 2016-03-13 15:51     ` Andrew Lunn
  2016-03-13 18:48       ` Vivien Didelot
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Lunn @ 2016-03-13 15:51 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: David Miller, netdev, Florian Fainelli

On Sun, Mar 13, 2016 at 03:10:13AM -0400, Vivien Didelot wrote:
> Andrew Lunn <andrew@lunn.ch> writes:
> 
> > All the drivers support multiple chips, but mv88e6123_61_65 is the
> > only one that reflects this in its naming. Change it to be consistent
> > with the other drivers.
> >
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> 
> Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> 
> When mv88e6xxx will become a driver by its own supporting different
> devices, it'll be good to rename it to a reference driver as well, say
> mv88e6352.

In device tree land, the convention is to use the lowest version
number supported. So it will probably be called mv88e6085.c.

> 
> > diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
> > new file mode 100644
> > index 000000000000..69a6f79dcb10
> > --- /dev/null
> > +++ b/drivers/net/dsa/mv88e6123.c
> > @@ -0,0 +1,124 @@
> 
> Note that to avoid the big diff above, you can use the -M option of
> git-format-patch to detect file renames.

Yes, i always forget that. I wounder why it is not turned on by
default? Maybe older versions of patch do not understand it?

	 Andrew

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

* Re: [PATCH net-next 1/5] dsa: Rename mv88e6123_61_65 to mv88e6123 to be consistent
  2016-03-13 15:51     ` Andrew Lunn
@ 2016-03-13 18:48       ` Vivien Didelot
  0 siblings, 0 replies; 12+ messages in thread
From: Vivien Didelot @ 2016-03-13 18:48 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> On Sun, Mar 13, 2016 at 03:10:13AM -0400, Vivien Didelot wrote:
>> Andrew Lunn <andrew@lunn.ch> writes:
>> 
>> > All the drivers support multiple chips, but mv88e6123_61_65 is the
>> > only one that reflects this in its naming. Change it to be consistent
>> > with the other drivers.
>> >
>> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
>> 
>> Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> 
>> When mv88e6xxx will become a driver by its own supporting different
>> devices, it'll be good to rename it to a reference driver as well, say
>> mv88e6352.

Ha, good to know, I didn't know about such convention. I like that.

> In device tree land, the convention is to use the lowest version
> number supported. So it will probably be called mv88e6085.c.
>
>> 
>> > diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
>> > new file mode 100644
>> > index 000000000000..69a6f79dcb10
>> > --- /dev/null
>> > +++ b/drivers/net/dsa/mv88e6123.c
>> > @@ -0,0 +1,124 @@
>> 
>> Note that to avoid the big diff above, you can use the -M option of
>> git-format-patch to detect file renames.
>
> Yes, i always forget that. I wounder why it is not turned on by
> default? Maybe older versions of patch do not understand it?

I'm not sure neither. Hopefully it is now supported by git show, diff,
format-patch and send-email subcommands.

Thanks,
Vivien

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

* Re: [PATCH net-next 0/5] DSA cleanup and fixes
  2016-03-11 23:01 [PATCH net-next 0/5] DSA cleanup and fixes Andrew Lunn
                   ` (4 preceding siblings ...)
  2016-03-11 23:01 ` [PATCH net-next 5/5] phy: fixed: Fix removal of phys Andrew Lunn
@ 2016-03-14 19:44 ` David Miller
  5 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2016-03-14 19:44 UTC (permalink / raw)
  To: andrew; +Cc: netdev, f.fainelli, vivien.didelot

From: Andrew Lunn <andrew@lunn.ch>
Date: Sat, 12 Mar 2016 00:01:35 +0100

> The RFC patchset for re-architecturing DSA probing contains a few
> standalone patches, either clean up or fixes. This pulls them out for
> submission.

Series applied, thanks Andrew.

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

end of thread, other threads:[~2016-03-14 19:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-11 23:01 [PATCH net-next 0/5] DSA cleanup and fixes Andrew Lunn
2016-03-11 23:01 ` [PATCH net-next 1/5] dsa: Rename mv88e6123_61_65 to mv88e6123 to be consistent Andrew Lunn
2016-03-13  7:10   ` Vivien Didelot
2016-03-13 15:51     ` Andrew Lunn
2016-03-13 18:48       ` Vivien Didelot
2016-03-11 23:01 ` [PATCH net-next 2/5] dsa: slave: Don't reference NULL pointer during phy_disconnect Andrew Lunn
2016-03-11 23:01 ` [PATCH net-next 3/5] dsa: Destroy fixed link phys after the phy has been disconnected Andrew Lunn
2016-03-11 23:01 ` [PATCH net-next 4/5] dsa: dsa: Fix freeing of fixed-phys from user ports Andrew Lunn
2016-03-11 23:01 ` [PATCH net-next 5/5] phy: fixed: Fix removal of phys Andrew Lunn
2016-03-11 23:18   ` Florian Fainelli
2016-03-12 17:24     ` Andrew Lunn
2016-03-14 19:44 ` [PATCH net-next 0/5] DSA cleanup and 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).