[PATCH net-next v3 00/10] net: dsa: Support for pdata in dsa2

[PATCH net-next v3 00/10] net: dsa: Support for pdata in dsa2

[Florian Fainelli]

Hi all,

This is not exactly new, and was sent before, although back then, I did not
have an user of the pre-declared MDIO board information, but now we do. Note
that I have additional changes queued up to have b53 register platform data for
MIPS bcm47xx and bcm63xx.

Yes I know that we should have the Orion platforms eventually be converted to
Device Tree, but until that happens, I don't want any remaining users of the
old "dsa" platform device (hence the previous DTS submissions for ARM/mvebu)
and, there will be platforms out there that most likely won't never see DT
coming their way (BCM47xx is almost 100% sure, BCM63xx maybe not in a distant
future).

We would probably want the whole series to be merged via David Miller's tree
to simplify things.

Greg, can you Ack/Nack patch 5 since it touched the core LDD?

Vivien, since some patches did change, I did not apply your Tested-by tag
to all patches.

Thanks!

Changes in v3:

- Tested EPROBE_DEFER from a mockup MDIO/DSA switch driver and everything
  is fine, once the driver finally probes we have access to platform data
  as expected

- added comment above dsa_port_is_valid() that port->name is mandatory
  for platform data cases

- added an extra check in dsa_parse_member() for a NULL pdata pointer

- fixed a bunch of checkpatch errors and warnings

Changes in v2:

- Rebased against latest net-next/master

- Moved dev_find_class() to device_find_class() into drivers/base/core.c

- Moved dev_to_net_device into net/core/dev.c

- Utilize dsa_chip_data directly instead of dsa_platform_data

- Augmented dsa_chip_data to be multi-CPU port ready

Changes from last submission (few months back):

- rebased against latest net-next

- do not introduce dsa2_platform_data which was overkill and was meant to
  allow us to do exaclty the same things with platform data and Device Tree
  we use the existing dsa_platform_data instead

- properly register MDIO devices when the MDIO bus is registered and associate
  platform_data with them

- add a change to the Orion platform code to demonstrate how this can be used

Thank you

Florian Fainelli (10):
  net: dsa: Pass device pointer to dsa_register_switch
  net: dsa: Make most functions take a dsa_port argument
  net: dsa: Suffix function manipulating device_node with _dn
  net: dsa: Move ports assignment closer to error checking
  drivers: base: Add device_find_class()
  net: dsa: Migrate to device_find_class()
  net: Relocate dev_to_net_device() into core
  net: dsa: Add support for platform data
  net: phy: Allow pre-declaration of MDIO devices
  ARM: orion: Register DSA switch as a MDIO device

 arch/arm/mach-orion5x/common.c               |   2 +-
 arch/arm/mach-orion5x/common.h               |   4 +-
 arch/arm/mach-orion5x/rd88f5181l-fxo-setup.c |   7 +-
 arch/arm/mach-orion5x/rd88f5181l-ge-setup.c  |   7 +-
 arch/arm/mach-orion5x/rd88f6183ap-ge-setup.c |   7 +-
 arch/arm/mach-orion5x/wnr854t-setup.c        |   2 +-
 arch/arm/mach-orion5x/wrt350n-v2-setup.c     |   7 +-
 arch/arm/plat-orion/common.c                 |  25 +++-
 arch/arm/plat-orion/include/plat/common.h    |   4 +-
 drivers/base/core.c                          |  19 +++
 drivers/net/dsa/b53/b53_common.c             |   2 +-
 drivers/net/dsa/mv88e6xxx/chip.c             |  11 +-
 drivers/net/dsa/qca8k.c                      |   2 +-
 drivers/net/phy/Makefile                     |   3 +-
 drivers/net/phy/mdio-boardinfo.c             |  86 +++++++++++++
 drivers/net/phy/mdio-boardinfo.h             |  19 +++
 drivers/net/phy/mdio_bus.c                   |   4 +
 drivers/net/phy/mdio_device.c                |  11 ++
 include/linux/device.h                       |   1 +
 include/linux/mdio.h                         |   3 +
 include/linux/mod_devicetable.h              |   1 +
 include/linux/netdevice.h                    |   2 +
 include/linux/phy.h                          |  19 +++
 include/net/dsa.h                            |   8 +-
 net/core/dev.c                               |  19 +++
 net/dsa/dsa.c                                |  53 ++------
 net/dsa/dsa2.c                               | 174 +++++++++++++++++++--------
 net/dsa/dsa_priv.h                           |   4 +-
 28 files changed, 365 insertions(+), 141 deletions(-)
 create mode 100644 drivers/net/phy/mdio-boardinfo.c
 create mode 100644 drivers/net/phy/mdio-boardinfo.h

-- 
2.9.3

[PATCH net-next v3 01/10] net: dsa: Pass device pointer to dsa_register_switch

[Florian Fainelli]

In preparation for allowing dsa_register_switch() to be supplied with
device/platform data, pass down a struct device pointer instead of a
struct device_node.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c |  2 +-
 drivers/net/dsa/mv88e6xxx/chip.c | 11 ++++++-----
 drivers/net/dsa/qca8k.c          |  2 +-
 include/net/dsa.h                |  2 +-
 net/dsa/dsa2.c                   |  7 ++++---
 5 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 5102a3701a1a..7179eed9ee6d 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1882,7 +1882,7 @@ int b53_switch_register(struct b53_device *dev)
 
 	pr_info("found switch: %s, rev %i\n", dev->name, dev->core_rev);
 
-	return dsa_register_switch(dev->ds, dev->ds->dev->of_node);
+	return dsa_register_switch(dev->ds, dev->ds->dev);
 }
 EXPORT_SYMBOL(b53_switch_register);
 
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 987b2dbbd35a..3238a4752b98 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4421,8 +4421,7 @@ static struct dsa_switch_driver mv88e6xxx_switch_drv = {
 	.ops			= &mv88e6xxx_switch_ops,
 };
 
-static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip,
-				     struct device_node *np)
+static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
 {
 	struct device *dev = chip->dev;
 	struct dsa_switch *ds;
@@ -4437,7 +4436,7 @@ static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip,
 
 	dev_set_drvdata(dev, ds);
 
-	return dsa_register_switch(ds, np);
+	return dsa_register_switch(ds, dev);
 }
 
 static void mv88e6xxx_unregister_switch(struct mv88e6xxx_chip *chip)
@@ -4521,9 +4520,11 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	if (err)
 		goto out_g2_irq;
 
-	err = mv88e6xxx_register_switch(chip, np);
-	if (err)
+	err = mv88e6xxx_register_switch(chip);
+	if (err) {
+		mv88e6xxx_mdio_unregister(chip);
 		goto out_mdio;
+	}
 
 	return 0;
 
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 54d270d59eb0..c084aa484d2b 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -964,7 +964,7 @@ qca8k_sw_probe(struct mdio_device *mdiodev)
 	mutex_init(&priv->reg_mutex);
 	dev_set_drvdata(&mdiodev->dev, priv);
 
-	return dsa_register_switch(priv->ds, priv->ds->dev->of_node);
+	return dsa_register_switch(priv->ds, &mdiodev->dev);
 }
 
 static void
diff --git a/include/net/dsa.h b/include/net/dsa.h
index b94d1f2ef912..16a502a6c26a 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -403,7 +403,7 @@ static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
 }
 
 void dsa_unregister_switch(struct dsa_switch *ds);
-int dsa_register_switch(struct dsa_switch *ds, struct device_node *np);
+int dsa_register_switch(struct dsa_switch *ds, struct device *dev);
 #ifdef CONFIG_PM_SLEEP
 int dsa_switch_suspend(struct dsa_switch *ds);
 int dsa_switch_resume(struct dsa_switch *ds);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 42a41d84053c..4170f7ea8e28 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -579,8 +579,9 @@ static struct device_node *dsa_get_ports(struct dsa_switch *ds,
 	return ports;
 }
 
-static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
+static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
 {
+	struct device_node *np = dev->of_node;
 	struct device_node *ports = dsa_get_ports(ds, np);
 	struct dsa_switch_tree *dst;
 	u32 tree, index;
@@ -660,12 +661,12 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
 	return err;
 }
 
-int dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
+int dsa_register_switch(struct dsa_switch *ds, struct device *dev)
 {
 	int err;
 
 	mutex_lock(&dsa2_mutex);
-	err = _dsa_register_switch(ds, np);
+	err = _dsa_register_switch(ds, dev);
 	mutex_unlock(&dsa2_mutex);
 
 	return err;
-- 
2.9.3

[PATCH net-next v3 02/10] net: dsa: Make most functions take a dsa_port argument

[Florian Fainelli]

In preparation for allowing platform data, and therefore no valid
device_node pointer, make most DSA functions takes a pointer to a
dsa_port structure whenever possible. While at it, introduce a
dsa_port_is_valid() helper function which checks whether port->dn is
NULL or not at the moment.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa.c      | 15 ++++++++------
 net/dsa/dsa2.c     | 61 +++++++++++++++++++++++++++++-------------------------
 net/dsa/dsa_priv.h |  4 ++--
 3 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index fd532487dfdf..2306d1b87c83 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -110,8 +110,9 @@ dsa_switch_probe(struct device *parent, struct device *host_dev, int sw_addr,
 
 /* basic switch operations **************************************************/
 int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
-		      struct device_node *port_dn, int port)
+		      struct dsa_port *dport, int port)
 {
+	struct device_node *port_dn = dport->dn;
 	struct phy_device *phydev;
 	int ret, mode;
 
@@ -141,15 +142,15 @@ int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
 
 static int dsa_cpu_dsa_setups(struct dsa_switch *ds, struct device *dev)
 {
-	struct device_node *port_dn;
+	struct dsa_port *dport;
 	int ret, port;
 
 	for (port = 0; port < DSA_MAX_PORTS; port++) {
 		if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
 			continue;
 
-		port_dn = ds->ports[port].dn;
-		ret = dsa_cpu_dsa_setup(ds, dev, port_dn, port);
+		dport = &ds->ports[port];
+		ret = dsa_cpu_dsa_setup(ds, dev, dport, port);
 		if (ret)
 			return ret;
 	}
@@ -366,8 +367,10 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
 	return ds;
 }
 
-void dsa_cpu_dsa_destroy(struct device_node *port_dn)
+void dsa_cpu_dsa_destroy(struct dsa_port *port)
 {
+	struct device_node *port_dn = port->dn;
+
 	if (of_phy_is_fixed_link(port_dn))
 		of_phy_deregister_fixed_link(port_dn);
 }
@@ -393,7 +396,7 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
 	for (port = 0; port < DSA_MAX_PORTS; port++) {
 		if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
 			continue;
-		dsa_cpu_dsa_destroy(ds->ports[port].dn);
+		dsa_cpu_dsa_destroy(&ds->ports[port]);
 
 		/* Clearing a bit which is not set does no harm */
 		ds->cpu_port_mask |= ~(1 << port);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 4170f7ea8e28..6e3675220fef 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -79,14 +79,19 @@ static void dsa_dst_del_ds(struct dsa_switch_tree *dst,
 	kref_put(&dst->refcount, dsa_free_dst);
 }
 
-static bool dsa_port_is_dsa(struct device_node *port)
+static bool dsa_port_is_valid(struct dsa_port *port)
 {
-	return !!of_parse_phandle(port, "link", 0);
+	return !!port->dn;
 }
 
-static bool dsa_port_is_cpu(struct device_node *port)
+static bool dsa_port_is_dsa(struct dsa_port *port)
 {
-	return !!of_parse_phandle(port, "ethernet", 0);
+	return !!of_parse_phandle(port->dn, "link", 0);
+}
+
+static bool dsa_port_is_cpu(struct dsa_port *port)
+{
+	return !!of_parse_phandle(port->dn, "ethernet", 0);
 }
 
 static bool dsa_ds_find_port(struct dsa_switch *ds,
@@ -120,7 +125,7 @@ static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
 
 static int dsa_port_complete(struct dsa_switch_tree *dst,
 			     struct dsa_switch *src_ds,
-			     struct device_node *port,
+			     struct dsa_port *port,
 			     u32 src_port)
 {
 	struct device_node *link;
@@ -128,7 +133,7 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
 	struct dsa_switch *dst_ds;
 
 	for (index = 0;; index++) {
-		link = of_parse_phandle(port, "link", index);
+		link = of_parse_phandle(port->dn, "link", index);
 		if (!link)
 			break;
 
@@ -151,13 +156,13 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
  */
 static int dsa_ds_complete(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 {
-	struct device_node *port;
+	struct dsa_port *port;
 	u32 index;
 	int err;
 
 	for (index = 0; index < DSA_MAX_PORTS; index++) {
-		port = ds->ports[index].dn;
-		if (!port)
+		port = &ds->ports[index];
+		if (!dsa_port_is_valid(port))
 			continue;
 
 		if (!dsa_port_is_dsa(port))
@@ -197,7 +202,7 @@ static int dsa_dst_complete(struct dsa_switch_tree *dst)
 	return 0;
 }
 
-static int dsa_dsa_port_apply(struct device_node *port, u32 index,
+static int dsa_dsa_port_apply(struct dsa_port *port, u32 index,
 			      struct dsa_switch *ds)
 {
 	int err;
@@ -212,13 +217,13 @@ static int dsa_dsa_port_apply(struct device_node *port, u32 index,
 	return 0;
 }
 
-static void dsa_dsa_port_unapply(struct device_node *port, u32 index,
+static void dsa_dsa_port_unapply(struct dsa_port *port, u32 index,
 				 struct dsa_switch *ds)
 {
 	dsa_cpu_dsa_destroy(port);
 }
 
-static int dsa_cpu_port_apply(struct device_node *port, u32 index,
+static int dsa_cpu_port_apply(struct dsa_port *port, u32 index,
 			      struct dsa_switch *ds)
 {
 	int err;
@@ -235,7 +240,7 @@ static int dsa_cpu_port_apply(struct device_node *port, u32 index,
 	return 0;
 }
 
-static void dsa_cpu_port_unapply(struct device_node *port, u32 index,
+static void dsa_cpu_port_unapply(struct dsa_port *port, u32 index,
 				 struct dsa_switch *ds)
 {
 	dsa_cpu_dsa_destroy(port);
@@ -243,13 +248,13 @@ static void dsa_cpu_port_unapply(struct device_node *port, u32 index,
 
 }
 
-static int dsa_user_port_apply(struct device_node *port, u32 index,
+static int dsa_user_port_apply(struct dsa_port *port, u32 index,
 			       struct dsa_switch *ds)
 {
 	const char *name;
 	int err;
 
-	name = of_get_property(port, "label", NULL);
+	name = of_get_property(port->dn, "label", NULL);
 	if (!name)
 		name = "eth%d";
 
@@ -263,7 +268,7 @@ static int dsa_user_port_apply(struct device_node *port, u32 index,
 	return 0;
 }
 
-static void dsa_user_port_unapply(struct device_node *port, u32 index,
+static void dsa_user_port_unapply(struct dsa_port *port, u32 index,
 				  struct dsa_switch *ds)
 {
 	if (ds->ports[index].netdev) {
@@ -275,7 +280,7 @@ static void dsa_user_port_unapply(struct device_node *port, u32 index,
 
 static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 {
-	struct device_node *port;
+	struct dsa_port *port;
 	u32 index;
 	int err;
 
@@ -309,8 +314,8 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 	}
 
 	for (index = 0; index < DSA_MAX_PORTS; index++) {
-		port = ds->ports[index].dn;
-		if (!port)
+		port = &ds->ports[index];
+		if (!dsa_port_is_valid(port))
 			continue;
 
 		if (dsa_port_is_dsa(port)) {
@@ -337,12 +342,12 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 
 static void dsa_ds_unapply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 {
-	struct device_node *port;
+	struct dsa_port *port;
 	u32 index;
 
 	for (index = 0; index < DSA_MAX_PORTS; index++) {
-		port = ds->ports[index].dn;
-		if (!port)
+		port = &ds->ports[index];
+		if (!dsa_port_is_valid(port))
 			continue;
 
 		if (dsa_port_is_dsa(port)) {
@@ -426,7 +431,7 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
 	dst->applied = false;
 }
 
-static int dsa_cpu_parse(struct device_node *port, u32 index,
+static int dsa_cpu_parse(struct dsa_port *port, u32 index,
 			 struct dsa_switch_tree *dst,
 			 struct dsa_switch *ds)
 {
@@ -434,7 +439,7 @@ static int dsa_cpu_parse(struct device_node *port, u32 index,
 	struct net_device *ethernet_dev;
 	struct device_node *ethernet;
 
-	ethernet = of_parse_phandle(port, "ethernet", 0);
+	ethernet = of_parse_phandle(port->dn, "ethernet", 0);
 	if (!ethernet)
 		return -EINVAL;
 
@@ -467,13 +472,13 @@ static int dsa_cpu_parse(struct device_node *port, u32 index,
 
 static int dsa_ds_parse(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 {
-	struct device_node *port;
+	struct dsa_port *port;
 	u32 index;
 	int err;
 
 	for (index = 0; index < DSA_MAX_PORTS; index++) {
-		port = ds->ports[index].dn;
-		if (!port)
+		port = &ds->ports[index];
+		if (!dsa_port_is_valid(port))
 			continue;
 
 		if (dsa_port_is_cpu(port)) {
@@ -534,7 +539,7 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
 		 * to have access to a correct value, just like what
 		 * net/dsa/dsa.c::dsa_switch_setup_one does.
 		 */
-		if (!dsa_port_is_cpu(port))
+		if (!dsa_port_is_cpu(&ds->ports[reg]))
 			ds->enabled_port_mask |= 1 << reg;
 	}
 
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 7e3385ec73f4..a015ec97c289 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -50,8 +50,8 @@ struct dsa_slave_priv {
 
 /* dsa.c */
 int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
-		      struct device_node *port_dn, int port);
-void dsa_cpu_dsa_destroy(struct device_node *port_dn);
+		      struct dsa_port *dport, int port);
+void dsa_cpu_dsa_destroy(struct dsa_port *dport);
 const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol);
 int dsa_cpu_port_ethtool_setup(struct dsa_switch *ds);
 void dsa_cpu_port_ethtool_restore(struct dsa_switch *ds);
-- 
2.9.3

[PATCH net-next v3 03/10] net: dsa: Suffix function manipulating device_node with _dn

[Florian Fainelli]

Make it clear that these functions take a device_node structure pointer

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa2.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 6e3675220fef..04ab62251fe3 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -94,8 +94,8 @@ static bool dsa_port_is_cpu(struct dsa_port *port)
 	return !!of_parse_phandle(port->dn, "ethernet", 0);
 }
 
-static bool dsa_ds_find_port(struct dsa_switch *ds,
-			     struct device_node *port)
+static bool dsa_ds_find_port_dn(struct dsa_switch *ds,
+				struct device_node *port)
 {
 	u32 index;
 
@@ -105,8 +105,8 @@ static bool dsa_ds_find_port(struct dsa_switch *ds,
 	return false;
 }
 
-static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
-					    struct device_node *port)
+static struct dsa_switch *dsa_dst_find_port_dn(struct dsa_switch_tree *dst,
+					       struct device_node *port)
 {
 	struct dsa_switch *ds;
 	u32 index;
@@ -116,7 +116,7 @@ static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
 		if (!ds)
 			continue;
 
-		if (dsa_ds_find_port(ds, port))
+		if (dsa_ds_find_port_dn(ds, port))
 			return ds;
 	}
 
@@ -137,7 +137,7 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
 		if (!link)
 			break;
 
-		dst_ds = dsa_dst_find_port(dst, link);
+		dst_ds = dsa_dst_find_port_dn(dst, link);
 		of_node_put(link);
 
 		if (!dst_ds)
@@ -546,7 +546,7 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
 	return 0;
 }
 
-static int dsa_parse_member(struct device_node *np, u32 *tree, u32 *index)
+static int dsa_parse_member_dn(struct device_node *np, u32 *tree, u32 *index)
 {
 	int err;
 
@@ -592,7 +592,7 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
 	u32 tree, index;
 	int i, err;
 
-	err = dsa_parse_member(np, &tree, &index);
+	err = dsa_parse_member_dn(np, &tree, &index);
 	if (err)
 		return err;
 
-- 
2.9.3

[PATCH net-next v3 04/10] net: dsa: Move ports assignment closer to error checking

[Florian Fainelli]

Move the assignment of ports in _dsa_register_switch() closer to where
it is checked, no functional change. Re-order declarations to be
preserve the inverted christmas tree style.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 04ab62251fe3..cd91070b5467 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -587,8 +587,8 @@ static struct device_node *dsa_get_ports(struct dsa_switch *ds,
 static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
 {
 	struct device_node *np = dev->of_node;
-	struct device_node *ports = dsa_get_ports(ds, np);
 	struct dsa_switch_tree *dst;
+	struct device_node *ports;
 	u32 tree, index;
 	int i, err;
 
@@ -596,6 +596,7 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
 	if (err)
 		return err;
 
+	ports = dsa_get_ports(ds, np);
 	if (IS_ERR(ports))
 		return PTR_ERR(ports);
 
-- 
2.9.3

[PATCH net-next v3 05/10] drivers: base: Add device_find_class()

[Florian Fainelli]

Add a helper function to lookup a device reference given a class name.
This is a preliminary patch to remove adhoc code from net/dsa/dsa.c and
make it more generic.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/base/core.c    | 19 +++++++++++++++++++
 include/linux/device.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 020ea7f05520..3dd6047c10d8 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2065,6 +2065,25 @@ struct device *device_find_child(struct device *parent, void *data,
 }
 EXPORT_SYMBOL_GPL(device_find_child);
 
+static int dev_is_class(struct device *dev, void *class)
+{
+	if (dev->class != NULL && !strcmp(dev->class->name, class))
+		return 1;
+
+	return 0;
+}
+
+struct device *device_find_class(struct device *parent, char *class)
+{
+	if (dev_is_class(parent, class)) {
+		get_device(parent);
+		return parent;
+	}
+
+	return device_find_child(parent, class, dev_is_class);
+}
+EXPORT_SYMBOL_GPL(device_find_class);
+
 int __init devices_init(void)
 {
 	devices_kset = kset_create_and_add("devices", &device_uevent_ops, NULL);
diff --git a/include/linux/device.h b/include/linux/device.h
index 491b4c0ca633..8d37f5ecb972 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1120,6 +1120,7 @@ extern int device_for_each_child_reverse(struct device *dev, void *data,
 		     int (*fn)(struct device *dev, void *data));
 extern struct device *device_find_child(struct device *dev, void *data,
 				int (*match)(struct device *dev, void *data));
+extern struct device *device_find_class(struct device *parent, char *class);
 extern int device_rename(struct device *dev, const char *new_name);
 extern int device_move(struct device *dev, struct device *new_parent,
 		       enum dpm_order dpm_order);
-- 
2.9.3

[PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Florian Fainelli]

Now that the base device driver code provides an identical
implementation of dev_find_class() utilize device_find_class() instead
of our own version of it.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa.c | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 2306d1b87c83..77fa4c4f5828 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -455,29 +455,11 @@ EXPORT_SYMBOL_GPL(dsa_switch_resume);
 #endif
 
 /* platform driver init and cleanup *****************************************/
-static int dev_is_class(struct device *dev, void *class)
-{
-	if (dev->class != NULL && !strcmp(dev->class->name, class))
-		return 1;
-
-	return 0;
-}
-
-static struct device *dev_find_class(struct device *parent, char *class)
-{
-	if (dev_is_class(parent, class)) {
-		get_device(parent);
-		return parent;
-	}
-
-	return device_find_child(parent, class, dev_is_class);
-}
-
 struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev)
 {
 	struct device *d;
 
-	d = dev_find_class(dev, "mdio_bus");
+	d = device_find_class(dev, "mdio_bus");
 	if (d != NULL) {
 		struct mii_bus *bus;
 
@@ -495,7 +477,7 @@ static struct net_device *dev_to_net_device(struct device *dev)
 {
 	struct device *d;
 
-	d = dev_find_class(dev, "net");
+	d = device_find_class(dev, "net");
 	if (d != NULL) {
 		struct net_device *nd;
 
-- 
2.9.3

[PATCH net-next v3 07/10] net: Relocate dev_to_net_device() into core

[Florian Fainelli]

dev_to_net_device() is moved from net/dsa/dsa.c to net/core/dev.c since
it going to be used by net/dsa/dsa2.c and the namespace of the function
justifies making it available to other users potentially.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/linux/netdevice.h |  2 ++
 net/core/dev.c            | 19 +++++++++++++++++++
 net/dsa/dsa.c             | 18 ------------------
 3 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 97ae0ac513ee..6d021c37b774 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4390,4 +4390,6 @@ do {								\
 #define PTYPE_HASH_SIZE	(16)
 #define PTYPE_HASH_MASK	(PTYPE_HASH_SIZE - 1)
 
+struct net_device *dev_to_net_device(struct device *dev);
+
 #endif	/* _LINUX_NETDEVICE_H */
diff --git a/net/core/dev.c b/net/core/dev.c
index ad5959e56116..7547e2ccc06b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8128,6 +8128,25 @@ const char *netdev_drivername(const struct net_device *dev)
 	return empty;
 }
 
+struct net_device *dev_to_net_device(struct device *dev)
+{
+	struct device *d;
+
+	d = device_find_class(dev, "net");
+	if (d) {
+		struct net_device *nd;
+
+		nd = to_net_dev(d);
+		dev_hold(nd);
+		put_device(d);
+
+		return nd;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(dev_to_net_device);
+
 static void __netdev_printk(const char *level, const struct net_device *dev,
 			    struct va_format *vaf)
 {
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 77fa4c4f5828..6c264f92fec5 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -473,24 +473,6 @@ struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(dsa_host_dev_to_mii_bus);
 
-static struct net_device *dev_to_net_device(struct device *dev)
-{
-	struct device *d;
-
-	d = device_find_class(dev, "net");
-	if (d != NULL) {
-		struct net_device *nd;
-
-		nd = to_net_dev(d);
-		dev_hold(nd);
-		put_device(d);
-
-		return nd;
-	}
-
-	return NULL;
-}
-
 #ifdef CONFIG_OF
 static int dsa_of_setup_routing_table(struct dsa_platform_data *pd,
 					struct dsa_chip_data *cd,
-- 
2.9.3

[PATCH net-next v3 08/10] net: dsa: Add support for platform data

[Florian Fainelli]

Allow drivers to use the new DSA API with platform data. Most of the
code in net/dsa/dsa2.c does not rely so much on device_nodes and can get
the same information from platform_data instead.

We purposely do not support distributed configurations with platform
data, so drivers should be providing a pointer to a 'struct
dsa_chip_data' structure if they wish to communicate per-port layout.

Multiple CPUs port could potentially be supported and dsa_chip_data is
extended to receive up to one reference to an upstream network device
per port described by a dsa_chip_data structure.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/net/dsa.h |   6 ++++
 net/dsa/dsa2.c    | 101 ++++++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 89 insertions(+), 18 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 16a502a6c26a..491008792e4d 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -42,6 +42,11 @@ struct dsa_chip_data {
 	struct device	*host_dev;
 	int		sw_addr;
 
+	/*
+	 * Reference to network devices
+	 */
+	struct device	*netdev[DSA_MAX_PORTS];
+
 	/* set to size of eeprom if supported by the switch */
 	int		eeprom_len;
 
@@ -140,6 +145,7 @@ struct dsa_switch_tree {
 };
 
 struct dsa_port {
+	const char		*name;
 	struct net_device	*netdev;
 	struct device_node	*dn;
 	unsigned int		ageing_time;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index cd91070b5467..598229b02fd3 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -79,19 +79,28 @@ static void dsa_dst_del_ds(struct dsa_switch_tree *dst,
 	kref_put(&dst->refcount, dsa_free_dst);
 }
 
+/* For platform data configurations, we need to have a valid name argument to
+ * differentiate a disabled port from an enabled one
+ */
 static bool dsa_port_is_valid(struct dsa_port *port)
 {
-	return !!port->dn;
+	return !!(port->dn || port->name);
 }
 
 static bool dsa_port_is_dsa(struct dsa_port *port)
 {
-	return !!of_parse_phandle(port->dn, "link", 0);
+	if (port->name && !strcmp(port->name, "dsa"))
+		return true;
+	else
+		return !!of_parse_phandle(port->dn, "link", 0);
 }
 
 static bool dsa_port_is_cpu(struct dsa_port *port)
 {
-	return !!of_parse_phandle(port->dn, "ethernet", 0);
+	if (port->name && !strcmp(port->name, "cpu"))
+		return true;
+	else
+		return !!of_parse_phandle(port->dn, "ethernet", 0);
 }
 
 static bool dsa_ds_find_port_dn(struct dsa_switch *ds,
@@ -251,10 +260,11 @@ static void dsa_cpu_port_unapply(struct dsa_port *port, u32 index,
 static int dsa_user_port_apply(struct dsa_port *port, u32 index,
 			       struct dsa_switch *ds)
 {
-	const char *name;
+	const char *name = port->name;
 	int err;
 
-	name = of_get_property(port->dn, "label", NULL);
+	if (port->dn)
+		name = of_get_property(port->dn, "label", NULL);
 	if (!name)
 		name = "eth%d";
 
@@ -439,11 +449,15 @@ static int dsa_cpu_parse(struct dsa_port *port, u32 index,
 	struct net_device *ethernet_dev;
 	struct device_node *ethernet;
 
-	ethernet = of_parse_phandle(port->dn, "ethernet", 0);
-	if (!ethernet)
-		return -EINVAL;
+	if (port->dn) {
+		ethernet = of_parse_phandle(port->dn, "ethernet", 0);
+		if (!ethernet)
+			return -EINVAL;
+		ethernet_dev = of_find_net_device_by_node(ethernet);
+	} else {
+		ethernet_dev = dev_to_net_device(ds->cd->netdev[index]);
+	}
 
-	ethernet_dev = of_find_net_device_by_node(ethernet);
 	if (!ethernet_dev)
 		return -EPROBE_DEFER;
 
@@ -546,6 +560,33 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
 	return 0;
 }
 
+static int dsa_parse_ports(struct dsa_chip_data *cd, struct dsa_switch *ds)
+{
+	bool valid_name_found = false;
+	unsigned int i;
+
+	for (i = 0; i < DSA_MAX_PORTS; i++) {
+		if (!cd->port_names[i])
+			continue;
+
+		ds->ports[i].name = cd->port_names[i];
+
+		/* Initialize enabled_port_mask now for drv->setup()
+		 * to have access to a correct value, just like what
+		 * net/dsa/dsa.c::dsa_switch_setup_one does.
+		 */
+		if (!dsa_port_is_cpu(&ds->ports[i]))
+			ds->enabled_port_mask |= 1 << i;
+
+		valid_name_found = true;
+	}
+
+	if (!valid_name_found && i == DSA_MAX_PORTS)
+		return -EINVAL;
+
+	return 0;
+}
+
 static int dsa_parse_member_dn(struct device_node *np, u32 *tree, u32 *index)
 {
 	int err;
@@ -570,6 +611,18 @@ static int dsa_parse_member_dn(struct device_node *np, u32 *tree, u32 *index)
 	return 0;
 }
 
+static int dsa_parse_member(struct dsa_chip_data *pd, u32 *tree, u32 *index)
+{
+	if (!pd)
+		return -ENODEV;
+
+	/* We do not support complex trees with dsa_chip_data */
+	*tree = 0;
+	*index = 0;
+
+	return 0;
+}
+
 static struct device_node *dsa_get_ports(struct dsa_switch *ds,
 					 struct device_node *np)
 {
@@ -586,23 +639,34 @@ static struct device_node *dsa_get_ports(struct dsa_switch *ds,
 
 static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
 {
+	struct dsa_chip_data *pdata = dev->platform_data;
 	struct device_node *np = dev->of_node;
 	struct dsa_switch_tree *dst;
 	struct device_node *ports;
 	u32 tree, index;
 	int i, err;
 
-	err = dsa_parse_member_dn(np, &tree, &index);
-	if (err)
-		return err;
+	if (np) {
+		err = dsa_parse_member_dn(np, &tree, &index);
+		if (err)
+			return err;
 
-	ports = dsa_get_ports(ds, np);
-	if (IS_ERR(ports))
-		return PTR_ERR(ports);
+		ports = dsa_get_ports(ds, np);
+		if (IS_ERR(ports))
+			return PTR_ERR(ports);
 
-	err = dsa_parse_ports_dn(ports, ds);
-	if (err)
-		return err;
+		err = dsa_parse_ports_dn(ports, ds);
+		if (err)
+			return err;
+	} else {
+		err = dsa_parse_member(pdata, &tree, &index);
+		if (err)
+			return err;
+
+		err = dsa_parse_ports(pdata, ds);
+		if (err)
+			return err;
+	}
 
 	dst = dsa_get_dst(tree);
 	if (!dst) {
@@ -618,6 +682,7 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
 
 	ds->dst = dst;
 	ds->index = index;
+	ds->cd = pdata;
 
 	/* Initialize the routing table */
 	for (i = 0; i < DSA_MAX_SWITCHES; ++i)
-- 
2.9.3

[PATCH net-next v3 09/10] net: phy: Allow pre-declaration of MDIO devices

[Florian Fainelli]

Allow board support code to collect pre-declarations for MDIO devices by
registering them with mdiobus_register_board_info(). SPI and I2C buses
have a similar feature, we were missing this for MDIO devices, but this
is particularly useful for e.g: MDIO-connected switches which need to
provide their port layout (often board-specific) to a MDIO Ethernet
switch driver.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/Makefile         |  3 +-
 drivers/net/phy/mdio-boardinfo.c | 86 ++++++++++++++++++++++++++++++++++++++++
 drivers/net/phy/mdio-boardinfo.h | 19 +++++++++
 drivers/net/phy/mdio_bus.c       |  4 ++
 drivers/net/phy/mdio_device.c    | 11 +++++
 include/linux/mdio.h             |  3 ++
 include/linux/mod_devicetable.h  |  1 +
 include/linux/phy.h              | 19 +++++++++
 8 files changed, 145 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/phy/mdio-boardinfo.c
 create mode 100644 drivers/net/phy/mdio-boardinfo.h

diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 356859ac7c18..407b0b601ea8 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -1,6 +1,7 @@
 # Makefile for Linux PHY drivers and MDIO bus drivers
 
-libphy-y			:= phy.o phy_device.o mdio_bus.o mdio_device.o
+libphy-y			:= phy.o phy_device.o mdio_bus.o mdio_device.o \
+				   mdio-boardinfo.o
 libphy-$(CONFIG_SWPHY)		+= swphy.o
 libphy-$(CONFIG_LED_TRIGGER_PHY)	+= phy_led_triggers.o
 
diff --git a/drivers/net/phy/mdio-boardinfo.c b/drivers/net/phy/mdio-boardinfo.c
new file mode 100644
index 000000000000..6b988f77da08
--- /dev/null
+++ b/drivers/net/phy/mdio-boardinfo.c
@@ -0,0 +1,86 @@
+/*
+ * mdio-boardinfo - Collect pre-declarations for MDIO devices
+ *
+ * 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/kernel.h>
+#include <linux/slab.h>
+#include <linux/export.h>
+#include <linux/mutex.h>
+#include <linux/list.h>
+
+#include "mdio-boardinfo.h"
+
+static LIST_HEAD(mdio_board_list);
+static DEFINE_MUTEX(mdio_board_lock);
+
+/**
+ * mdiobus_setup_mdiodev_from_board_info - create and setup MDIO devices
+ * from pre-collected board specific MDIO information
+ * @mdiodev: MDIO device pointer
+ * Context: can sleep
+ */
+void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus)
+{
+	struct mdio_board_entry *be;
+	struct mdio_device *mdiodev;
+	struct mdio_board_info *bi;
+	int ret;
+
+	mutex_lock(&mdio_board_lock);
+	list_for_each_entry(be, &mdio_board_list, list) {
+		bi = &be->board_info;
+
+		if (strcmp(bus->id, bi->bus_id))
+			continue;
+
+		mdiodev = mdio_device_create(bus, bi->mdio_addr);
+		if (IS_ERR(mdiodev))
+			continue;
+
+		strncpy(mdiodev->modalias, bi->modalias,
+			sizeof(mdiodev->modalias));
+		mdiodev->bus_match = mdio_device_bus_match;
+		mdiodev->dev.platform_data = (void *)bi->platform_data;
+
+		ret = mdio_device_register(mdiodev);
+		if (ret) {
+			mdio_device_free(mdiodev);
+			continue;
+		}
+	}
+	mutex_unlock(&mdio_board_lock);
+}
+
+/**
+ * mdio_register_board_info - register MDIO devices for a given board
+ * @info: array of devices descriptors
+ * @n: number of descriptors provided
+ * Context: can sleep
+ *
+ * The board info passed can be marked with __initdata but be pointers
+ * such as platform_data etc. are copied as-is
+ */
+int mdiobus_register_board_info(const struct mdio_board_info *info,
+				unsigned int n)
+{
+	struct mdio_board_entry *be;
+	unsigned int i;
+
+	be = kcalloc(n, sizeof(*be), GFP_KERNEL);
+	if (!be)
+		return -ENOMEM;
+
+	for (i = 0; i < n; i++, be++, info++) {
+		memcpy(&be->board_info, info, sizeof(*info));
+		mutex_lock(&mdio_board_lock);
+		list_add_tail(&be->list, &mdio_board_list);
+		mutex_unlock(&mdio_board_lock);
+	}
+
+	return 0;
+}
diff --git a/drivers/net/phy/mdio-boardinfo.h b/drivers/net/phy/mdio-boardinfo.h
new file mode 100644
index 000000000000..00f98163e90e
--- /dev/null
+++ b/drivers/net/phy/mdio-boardinfo.h
@@ -0,0 +1,19 @@
+/*
+ * mdio-boardinfo.h - board info interface internal to the mdio_bus
+ * component
+ */
+
+#ifndef __MDIO_BOARD_INFO_H
+#define __MDIO_BOARD_INFO_H
+
+#include <linux/phy.h>
+#include <linux/mutex.h>
+
+struct mdio_board_entry {
+	struct list_head	list;
+	struct mdio_board_info	board_info;
+};
+
+void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus);
+
+#endif /* __MDIO_BOARD_INFO_H */
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 653d076eafe5..fa7d51f14869 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -41,6 +41,8 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/mdio.h>
 
+#include "mdio-boardinfo.h"
+
 int mdiobus_register_device(struct mdio_device *mdiodev)
 {
 	if (mdiodev->bus->mdio_map[mdiodev->addr])
@@ -343,6 +345,8 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
 		}
 	}
 
+	mdiobus_setup_mdiodev_from_board_info(bus);
+
 	bus->state = MDIOBUS_REGISTERED;
 	pr_info("%s: probed\n", bus->name);
 	return 0;
diff --git a/drivers/net/phy/mdio_device.c b/drivers/net/phy/mdio_device.c
index fc3aaaa36b1d..e24f28924af8 100644
--- a/drivers/net/phy/mdio_device.c
+++ b/drivers/net/phy/mdio_device.c
@@ -34,6 +34,17 @@ static void mdio_device_release(struct device *dev)
 	kfree(to_mdio_device(dev));
 }
 
+int mdio_device_bus_match(struct device *dev, struct device_driver *drv)
+{
+	struct mdio_device *mdiodev = to_mdio_device(dev);
+	struct mdio_driver *mdiodrv = to_mdio_driver(drv);
+
+	if (mdiodrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY)
+		return 0;
+
+	return strcmp(mdiodev->modalias, drv->name) == 0;
+}
+
 struct mdio_device *mdio_device_create(struct mii_bus *bus, int addr)
 {
 	struct mdio_device *mdiodev;
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index b6587a4b32e7..00a7f43c1482 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -10,6 +10,7 @@
 #define __LINUX_MDIO_H__
 
 #include <uapi/linux/mdio.h>
+#include <linux/mod_devicetable.h>
 
 struct mii_bus;
 
@@ -29,6 +30,7 @@ struct mdio_device {
 
 	const struct dev_pm_ops *pm_ops;
 	struct mii_bus *bus;
+	char modalias[MDIO_NAME_SIZE];
 
 	int (*bus_match)(struct device *dev, struct device_driver *drv);
 	void (*device_free)(struct mdio_device *mdiodev);
@@ -71,6 +73,7 @@ int mdio_device_register(struct mdio_device *mdiodev);
 void mdio_device_remove(struct mdio_device *mdiodev);
 int mdio_driver_register(struct mdio_driver *drv);
 void mdio_driver_unregister(struct mdio_driver *drv);
+int mdio_device_bus_match(struct device *dev, struct device_driver *drv);
 
 static inline bool mdio_phy_id_is_c45(int phy_id)
 {
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 8a57f0b1242d..8850fcaf50db 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -501,6 +501,7 @@ struct platform_device_id {
 	kernel_ulong_t driver_data;
 };
 
+#define MDIO_NAME_SIZE		32
 #define MDIO_MODULE_PREFIX	"mdio:"
 
 #define MDIO_ID_FMT "%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d"
diff --git a/include/linux/phy.h b/include/linux/phy.h
index f7d95f644eed..7745f7391d3e 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -882,6 +882,25 @@ void mdio_bus_exit(void);
 
 extern struct bus_type mdio_bus_type;
 
+struct mdio_board_info {
+	const char	*bus_id;
+	char		modalias[MDIO_NAME_SIZE];
+	int		mdio_addr;
+	const void	*platform_data;
+};
+
+#if IS_ENABLED(CONFIG_PHYLIB)
+int mdiobus_register_board_info(const struct mdio_board_info *info,
+				unsigned int n);
+#else
+static inline int mdiobus_register_board_info(const struct mdio_board_info *i,
+					      unsigned int n)
+{
+	return 0;
+}
+#endif
+
+
 /**
  * module_phy_driver() - Helper macro for registering PHY drivers
  * @__phy_drivers: array of PHY drivers to register
-- 
2.9.3

[PATCH net-next v3 10/10] ARM: orion: Register DSA switch as a MDIO device

[Florian Fainelli]

Utilize the ability to pass board specific MDIO bus information towards a
particular MDIO device thus allowing us to provide the per-port switch layout
to the Marvell 88E6XXX switch driver.

Since we would end-up with conflicting registration paths, do not register the
"dsa" platform device anymore.

Note that the MDIO devices registered by code in net/dsa/dsa2.c does not
parse a dsa_platform_data, but directly take a dsa_chip_data (specific
to a single switch chip), so we update the different call sites to pass
this structure down to orion_ge00_switch_init().

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm/mach-orion5x/common.c               |  2 +-
 arch/arm/mach-orion5x/common.h               |  4 ++--
 arch/arm/mach-orion5x/rd88f5181l-fxo-setup.c |  7 +------
 arch/arm/mach-orion5x/rd88f5181l-ge-setup.c  |  7 +------
 arch/arm/mach-orion5x/rd88f6183ap-ge-setup.c |  7 +------
 arch/arm/mach-orion5x/wnr854t-setup.c        |  2 +-
 arch/arm/mach-orion5x/wrt350n-v2-setup.c     |  7 +------
 arch/arm/plat-orion/common.c                 | 25 +++++++++++++++++++------
 arch/arm/plat-orion/include/plat/common.h    |  4 ++--
 9 files changed, 29 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-orion5x/common.c b/arch/arm/mach-orion5x/common.c
index 04910764c385..83a7ec4c16d0 100644
--- a/arch/arm/mach-orion5x/common.c
+++ b/arch/arm/mach-orion5x/common.c
@@ -105,7 +105,7 @@ void __init orion5x_eth_init(struct mv643xx_eth_platform_data *eth_data)
 /*****************************************************************************
  * Ethernet switch
  ****************************************************************************/
-void __init orion5x_eth_switch_init(struct dsa_platform_data *d)
+void __init orion5x_eth_switch_init(struct dsa_chip_data *d)
 {
 	orion_ge00_switch_init(d);
 }
diff --git a/arch/arm/mach-orion5x/common.h b/arch/arm/mach-orion5x/common.h
index 8a4115bd441d..efeffc6b4ebb 100644
--- a/arch/arm/mach-orion5x/common.h
+++ b/arch/arm/mach-orion5x/common.h
@@ -3,7 +3,7 @@
 
 #include <linux/reboot.h>
 
-struct dsa_platform_data;
+struct dsa_chip_data;
 struct mv643xx_eth_platform_data;
 struct mv_sata_platform_data;
 
@@ -41,7 +41,7 @@ void orion5x_setup_wins(void);
 void orion5x_ehci0_init(void);
 void orion5x_ehci1_init(void);
 void orion5x_eth_init(struct mv643xx_eth_platform_data *eth_data);
-void orion5x_eth_switch_init(struct dsa_platform_data *d);
+void orion5x_eth_switch_init(struct dsa_chip_data *d);
 void orion5x_i2c_init(void);
 void orion5x_sata_init(struct mv_sata_platform_data *sata_data);
 void orion5x_spi_init(void);
diff --git a/arch/arm/mach-orion5x/rd88f5181l-fxo-setup.c b/arch/arm/mach-orion5x/rd88f5181l-fxo-setup.c
index dccadf68ea2b..a3c1336d30c9 100644
--- a/arch/arm/mach-orion5x/rd88f5181l-fxo-setup.c
+++ b/arch/arm/mach-orion5x/rd88f5181l-fxo-setup.c
@@ -101,11 +101,6 @@ static struct dsa_chip_data rd88f5181l_fxo_switch_chip_data = {
 	.port_names[7]	= "lan3",
 };
 
-static struct dsa_platform_data __initdata rd88f5181l_fxo_switch_plat_data = {
-	.nr_chips	= 1,
-	.chip		= &rd88f5181l_fxo_switch_chip_data,
-};
-
 static void __init rd88f5181l_fxo_init(void)
 {
 	/*
@@ -120,7 +115,7 @@ static void __init rd88f5181l_fxo_init(void)
 	 */
 	orion5x_ehci0_init();
 	orion5x_eth_init(&rd88f5181l_fxo_eth_data);
-	orion5x_eth_switch_init(&rd88f5181l_fxo_switch_plat_data);
+	orion5x_eth_switch_init(&rd88f5181l_fxo_switch_chip_data);
 	orion5x_uart0_init();
 
 	mvebu_mbus_add_window_by_id(ORION_MBUS_DEVBUS_BOOT_TARGET,
diff --git a/arch/arm/mach-orion5x/rd88f5181l-ge-setup.c b/arch/arm/mach-orion5x/rd88f5181l-ge-setup.c
index affe5ec825de..252efe29bd1a 100644
--- a/arch/arm/mach-orion5x/rd88f5181l-ge-setup.c
+++ b/arch/arm/mach-orion5x/rd88f5181l-ge-setup.c
@@ -102,11 +102,6 @@ static struct dsa_chip_data rd88f5181l_ge_switch_chip_data = {
 	.port_names[7]	= "lan3",
 };
 
-static struct dsa_platform_data __initdata rd88f5181l_ge_switch_plat_data = {
-	.nr_chips	= 1,
-	.chip		= &rd88f5181l_ge_switch_chip_data,
-};
-
 static struct i2c_board_info __initdata rd88f5181l_ge_i2c_rtc = {
 	I2C_BOARD_INFO("ds1338", 0x68),
 };
@@ -125,7 +120,7 @@ static void __init rd88f5181l_ge_init(void)
 	 */
 	orion5x_ehci0_init();
 	orion5x_eth_init(&rd88f5181l_ge_eth_data);
-	orion5x_eth_switch_init(&rd88f5181l_ge_switch_plat_data);
+	orion5x_eth_switch_init(&rd88f5181l_ge_switch_chip_data);
 	orion5x_i2c_init();
 	orion5x_uart0_init();
 
diff --git a/arch/arm/mach-orion5x/rd88f6183ap-ge-setup.c b/arch/arm/mach-orion5x/rd88f6183ap-ge-setup.c
index 67ee8571b03c..f4f1dbe1d91d 100644
--- a/arch/arm/mach-orion5x/rd88f6183ap-ge-setup.c
+++ b/arch/arm/mach-orion5x/rd88f6183ap-ge-setup.c
@@ -40,11 +40,6 @@ static struct dsa_chip_data rd88f6183ap_ge_switch_chip_data = {
 	.port_names[5]	= "cpu",
 };
 
-static struct dsa_platform_data __initdata rd88f6183ap_ge_switch_plat_data = {
-	.nr_chips	= 1,
-	.chip		= &rd88f6183ap_ge_switch_chip_data,
-};
-
 static struct mtd_partition rd88f6183ap_ge_partitions[] = {
 	{
 		.name	= "kernel",
@@ -89,7 +84,7 @@ static void __init rd88f6183ap_ge_init(void)
 	 */
 	orion5x_ehci0_init();
 	orion5x_eth_init(&rd88f6183ap_ge_eth_data);
-	orion5x_eth_switch_init(&rd88f6183ap_ge_switch_plat_data);
+	orion5x_eth_switch_init(&rd88f6183ap_ge_switch_chip_data);
 	spi_register_board_info(rd88f6183ap_ge_spi_slave_info,
 				ARRAY_SIZE(rd88f6183ap_ge_spi_slave_info));
 	orion5x_spi_init();
diff --git a/arch/arm/mach-orion5x/wnr854t-setup.c b/arch/arm/mach-orion5x/wnr854t-setup.c
index 4dbcdbe1de7c..ac3376fc269f 100644
--- a/arch/arm/mach-orion5x/wnr854t-setup.c
+++ b/arch/arm/mach-orion5x/wnr854t-setup.c
@@ -124,7 +124,7 @@ static void __init wnr854t_init(void)
 	 * Configure peripherals.
 	 */
 	orion5x_eth_init(&wnr854t_eth_data);
-	orion5x_eth_switch_init(&wnr854t_switch_plat_data);
+	orion5x_eth_switch_init(&wnr854t_switch_chip_data);
 	orion5x_uart0_init();
 
 	mvebu_mbus_add_window_by_id(ORION_MBUS_DEVBUS_BOOT_TARGET,
diff --git a/arch/arm/mach-orion5x/wrt350n-v2-setup.c b/arch/arm/mach-orion5x/wrt350n-v2-setup.c
index a6a8c4648d74..9250bb2e429c 100644
--- a/arch/arm/mach-orion5x/wrt350n-v2-setup.c
+++ b/arch/arm/mach-orion5x/wrt350n-v2-setup.c
@@ -191,11 +191,6 @@ static struct dsa_chip_data wrt350n_v2_switch_chip_data = {
 	.port_names[7]	= "lan4",
 };
 
-static struct dsa_platform_data __initdata wrt350n_v2_switch_plat_data = {
-	.nr_chips	= 1,
-	.chip		= &wrt350n_v2_switch_chip_data,
-};
-
 static void __init wrt350n_v2_init(void)
 {
 	/*
@@ -210,7 +205,7 @@ static void __init wrt350n_v2_init(void)
 	 */
 	orion5x_ehci0_init();
 	orion5x_eth_init(&wrt350n_v2_eth_data);
-	orion5x_eth_switch_init(&wrt350n_v2_switch_plat_data);
+	orion5x_eth_switch_init(&wrt350n_v2_switch_chip_data);
 	orion5x_uart0_init();
 
 	mvebu_mbus_add_window_by_id(ORION_MBUS_DEVBUS_BOOT_TARGET,
diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index 272f49b2c68f..9255b6d67ba5 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -22,6 +22,7 @@
 #include <linux/platform_data/dma-mv_xor.h>
 #include <linux/platform_data/usb-ehci-orion.h>
 #include <plat/common.h>
+#include <linux/phy.h>
 
 /* Create a clkdev entry for a given device/clk */
 void __init orion_clkdev_add(const char *con_id, const char *dev_id,
@@ -470,15 +471,27 @@ void __init orion_ge11_init(struct mv643xx_eth_platform_data *eth_data,
 /*****************************************************************************
  * Ethernet switch
  ****************************************************************************/
-void __init orion_ge00_switch_init(struct dsa_platform_data *d)
+static __initconst const char *orion_ge00_mvmdio_bus_name = "orion-mii";
+static __initdata struct mdio_board_info
+		  orion_ge00_switch_board_info;
+
+void __init orion_ge00_switch_init(struct dsa_chip_data *d)
 {
-	int i;
+	struct mdio_board_info *bd;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(d->port_names); i++)
+		if (!strcmp(d->port_names[i], "cpu"))
+			break;
 
-	d->netdev = &orion_ge00.dev;
-	for (i = 0; i < d->nr_chips; i++)
-		d->chip[i].host_dev = &orion_ge_mvmdio.dev;
+	bd = &orion_ge00_switch_board_info;
+	bd->bus_id = orion_ge00_mvmdio_bus_name;
+	bd->mdio_addr = d->sw_addr;
+	d->netdev[i] = &orion_ge00.dev;
+	strcpy(bd->modalias, "mv88e6085");
+	bd->platform_data = d;
 
-	platform_device_register_data(NULL, "dsa", 0, d, sizeof(d));
+	mdiobus_register_board_info(&orion_ge00_switch_board_info, 1);
 }
 
 /*****************************************************************************
diff --git a/arch/arm/plat-orion/include/plat/common.h b/arch/arm/plat-orion/include/plat/common.h
index 9347f3c58a6d..3647d3b33c20 100644
--- a/arch/arm/plat-orion/include/plat/common.h
+++ b/arch/arm/plat-orion/include/plat/common.h
@@ -12,7 +12,7 @@
 #include <linux/mv643xx_eth.h>
 #include <linux/platform_data/usb-ehci-orion.h>
 
-struct dsa_platform_data;
+struct dsa_chip_data;
 struct mv_sata_platform_data;
 
 void __init orion_uart0_init(void __iomem *membase,
@@ -57,7 +57,7 @@ void __init orion_ge11_init(struct mv643xx_eth_platform_data *eth_data,
 			    unsigned long mapbase,
 			    unsigned long irq);
 
-void __init orion_ge00_switch_init(struct dsa_platform_data *d);
+void __init orion_ge00_switch_init(struct dsa_chip_data *d);
 
 void __init orion_i2c_init(unsigned long mapbase,
 			   unsigned long irq,
-- 
2.9.3

Re: [PATCH net-next v3 04/10] net: dsa: Move ports assignment closer to error checking

[Sergei Shtylyov]

Hello!

On 1/15/2017 12:47 AM, Florian Fainelli wrote:

> Move the assignment of ports in _dsa_register_switch() closer to where
> it is checked, no functional change. Re-order declarations to be

    "Be" not needed.

> preserve the inverted christmas tree style.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
[...]

MBR, Sergei

Re: [PATCH net-next v3 05/10] drivers: base: Add device_find_class()

[Greg KH]

On Sat, Jan 14, 2017 at 01:47:08PM -0800, Florian Fainelli wrote:
> Add a helper function to lookup a device reference given a class name.
> This is a preliminary patch to remove adhoc code from net/dsa/dsa.c and
> make it more generic.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/base/core.c    | 19 +++++++++++++++++++
>  include/linux/device.h |  1 +
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 020ea7f05520..3dd6047c10d8 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2065,6 +2065,25 @@ struct device *device_find_child(struct device *parent, void *data,
>  }
>  EXPORT_SYMBOL_GPL(device_find_child);
>  
> +static int dev_is_class(struct device *dev, void *class)
> +{
> +	if (dev->class != NULL && !strcmp(dev->class->name, class))
> +		return 1;
> +
> +	return 0;
> +}
> +
> +struct device *device_find_class(struct device *parent, char *class)

Why are you using the char * for a class, and not just a pointer to
"struct class"?  That seems to be the most logical one, no need to rely
on string comparisons here.

Also, what is this being used for?  You aren't trying to walk up the
device heirachy to find a specific "type" of device, are you?  If so,
ugh, I ranted about this in the past when the hyperv driver was trying
to do such a thing...

> +{
> +	if (dev_is_class(parent, class)) {
> +		get_device(parent);
> +		return parent;
> +	}
> +
> +	return device_find_child(parent, class, dev_is_class);

You are trying to find a peer device with the same parent that belongs
to a specific class?

Again, what is this being used for?

And all exported driver core functions should have full kerneldoc
information for them so that people know how to use them, and what the
constraints are (see device_find_child() as an example.)  Please do that
here as well because you are returning a pointer to a structure with the
reference count incremented, callers need to know that.

thanks,

greg k-h

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Greg KH]

On Sat, Jan 14, 2017 at 01:47:09PM -0800, Florian Fainelli wrote:
> Now that the base device driver code provides an identical
> implementation of dev_find_class() utilize device_find_class() instead
> of our own version of it.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  net/dsa/dsa.c | 22 ++--------------------
>  1 file changed, 2 insertions(+), 20 deletions(-)
> 
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 2306d1b87c83..77fa4c4f5828 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -455,29 +455,11 @@ EXPORT_SYMBOL_GPL(dsa_switch_resume);
>  #endif
>  
>  /* platform driver init and cleanup *****************************************/
> -static int dev_is_class(struct device *dev, void *class)
> -{
> -	if (dev->class != NULL && !strcmp(dev->class->name, class))
> -		return 1;
> -
> -	return 0;
> -}
> -
> -static struct device *dev_find_class(struct device *parent, char *class)
> -{
> -	if (dev_is_class(parent, class)) {
> -		get_device(parent);
> -		return parent;
> -	}
> -
> -	return device_find_child(parent, class, dev_is_class);
> -}
> -
>  struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev)
>  {
>  	struct device *d;
>  
> -	d = dev_find_class(dev, "mdio_bus");
> +	d = device_find_class(dev, "mdio_bus");
>  	if (d != NULL) {
>  		struct mii_bus *bus;

You want a peer of your device on a specific class?  What is this for?

> @@ -495,7 +477,7 @@ static struct net_device *dev_to_net_device(struct device *dev)
>  {
>  	struct device *d;
>  
> -	d = dev_find_class(dev, "net");
> +	d = device_find_class(dev, "net");
>  	if (d != NULL) {
>  		struct net_device *nd;

Again, huh?  What is the device heirachy here that is so odd that this
type of function is needed?

totally confused,

greg k-h

Re: [PATCH net-next v3 07/10] net: Relocate dev_to_net_device() into core

[Greg KH]

On Sat, Jan 14, 2017 at 01:47:10PM -0800, Florian Fainelli wrote:
> dev_to_net_device() is moved from net/dsa/dsa.c to net/core/dev.c since
> it going to be used by net/dsa/dsa2.c and the namespace of the function
> justifies making it available to other users potentially.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  include/linux/netdevice.h |  2 ++
>  net/core/dev.c            | 19 +++++++++++++++++++
>  net/dsa/dsa.c             | 18 ------------------
>  3 files changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 97ae0ac513ee..6d021c37b774 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -4390,4 +4390,6 @@ do {								\
>  #define PTYPE_HASH_SIZE	(16)
>  #define PTYPE_HASH_MASK	(PTYPE_HASH_SIZE - 1)
>  
> +struct net_device *dev_to_net_device(struct device *dev);
> +
>  #endif	/* _LINUX_NETDEVICE_H */
> diff --git a/net/core/dev.c b/net/core/dev.c
> index ad5959e56116..7547e2ccc06b 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -8128,6 +8128,25 @@ const char *netdev_drivername(const struct net_device *dev)
>  	return empty;
>  }
>  
> +struct net_device *dev_to_net_device(struct device *dev)
> +{
> +	struct device *d;
> +
> +	d = device_find_class(dev, "net");
> +	if (d) {
> +		struct net_device *nd;
> +
> +		nd = to_net_dev(d);
> +		dev_hold(nd);
> +		put_device(d);
> +
> +		return nd;
> +	}
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(dev_to_net_device);

This really isn't just a "struct device to net device cast" type
function, (otherwise a simple container_of() would work).  You are
walking the device tree and assuming it is in a specific order so that
this function works.  You better document the hell out of this,
otherwise people are going to try to use this and get very confused,
very quickly...

thanks,

greg k-h

Re: [PATCH net-next v3 00/10] net: dsa: Support for pdata in dsa2

[Greg KH]

On Sat, Jan 14, 2017 at 01:47:03PM -0800, Florian Fainelli wrote:
> Hi all,
> 
> This is not exactly new, and was sent before, although back then, I did not
> have an user of the pre-declared MDIO board information, but now we do. Note
> that I have additional changes queued up to have b53 register platform data for
> MIPS bcm47xx and bcm63xx.
> 
> Yes I know that we should have the Orion platforms eventually be converted to
> Device Tree, but until that happens, I don't want any remaining users of the
> old "dsa" platform device (hence the previous DTS submissions for ARM/mvebu)
> and, there will be platforms out there that most likely won't never see DT
> coming their way (BCM47xx is almost 100% sure, BCM63xx maybe not in a distant
> future).
> 
> We would probably want the whole series to be merged via David Miller's tree
> to simplify things.
> 
> Greg, can you Ack/Nack patch 5 since it touched the core LDD?

I've NAKed them for now, you need to describe what you are trying to do
here, as it doesn't make any sense to me at the moment.

thanks,

greg k-h

Re: [PATCH net-next v3 07/10] net: Relocate dev_to_net_device() into core

[Florian Fainelli]

On 01/15/2017 03:07 AM, Greg KH wrote:
> On Sat, Jan 14, 2017 at 01:47:10PM -0800, Florian Fainelli wrote:
>> dev_to_net_device() is moved from net/dsa/dsa.c to net/core/dev.c since
>> it going to be used by net/dsa/dsa2.c and the namespace of the function
>> justifies making it available to other users potentially.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  include/linux/netdevice.h |  2 ++
>>  net/core/dev.c            | 19 +++++++++++++++++++
>>  net/dsa/dsa.c             | 18 ------------------
>>  3 files changed, 21 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 97ae0ac513ee..6d021c37b774 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -4390,4 +4390,6 @@ do {								\
>>  #define PTYPE_HASH_SIZE	(16)
>>  #define PTYPE_HASH_MASK	(PTYPE_HASH_SIZE - 1)
>>  
>> +struct net_device *dev_to_net_device(struct device *dev);
>> +
>>  #endif	/* _LINUX_NETDEVICE_H */
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index ad5959e56116..7547e2ccc06b 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -8128,6 +8128,25 @@ const char *netdev_drivername(const struct net_device *dev)
>>  	return empty;
>>  }
>>  
>> +struct net_device *dev_to_net_device(struct device *dev)
>> +{
>> +	struct device *d;
>> +
>> +	d = device_find_class(dev, "net");
>> +	if (d) {
>> +		struct net_device *nd;
>> +
>> +		nd = to_net_dev(d);
>> +		dev_hold(nd);
>> +		put_device(d);
>> +
>> +		return nd;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(dev_to_net_device);
> 
> This really isn't just a "struct device to net device cast" type
> function, (otherwise a simple container_of() would work).  You are
> walking the device tree and assuming it is in a specific order so that
> this function works.  You better document the hell out of this,
> otherwise people are going to try to use this and get very confused,
> very quickly...

Fair enough. Does that make it clearer how the device_find_class() is
used though? Maybe device_find_class() should be named
device_find_by_class_name() instead?
-- 
Florian

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Florian Fainelli]

On 01/15/2017 03:06 AM, Greg KH wrote:
> On Sat, Jan 14, 2017 at 01:47:09PM -0800, Florian Fainelli wrote:
>> Now that the base device driver code provides an identical
>> implementation of dev_find_class() utilize device_find_class() instead
>> of our own version of it.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  net/dsa/dsa.c | 22 ++--------------------
>>  1 file changed, 2 insertions(+), 20 deletions(-)
>>
>> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
>> index 2306d1b87c83..77fa4c4f5828 100644
>> --- a/net/dsa/dsa.c
>> +++ b/net/dsa/dsa.c
>> @@ -455,29 +455,11 @@ EXPORT_SYMBOL_GPL(dsa_switch_resume);
>>  #endif
>>  
>>  /* platform driver init and cleanup *****************************************/
>> -static int dev_is_class(struct device *dev, void *class)
>> -{
>> -	if (dev->class != NULL && !strcmp(dev->class->name, class))
>> -		return 1;
>> -
>> -	return 0;
>> -}
>> -
>> -static struct device *dev_find_class(struct device *parent, char *class)
>> -{
>> -	if (dev_is_class(parent, class)) {
>> -		get_device(parent);
>> -		return parent;
>> -	}
>> -
>> -	return device_find_child(parent, class, dev_is_class);
>> -}
>> -
>>  struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev)
>>  {
>>  	struct device *d;
>>  
>> -	d = dev_find_class(dev, "mdio_bus");
>> +	d = device_find_class(dev, "mdio_bus");
>>  	if (d != NULL) {
>>  		struct mii_bus *bus;
> 
> You want a peer of your device on a specific class?  What is this for?

It's not a peer of our device, it's a separate device reference from the
one looked up in the "net" class. In the classic, and now deprecated DSA
device driver model, a "dsa" platform device would represent one or more
Ethernet switches, connected via a MDIO bus (this reference above), and
one Ethernet device (the CPU/host/management interface). This was
completely violating the Linux device driver model and imposed
limitations on what bus would be used, and we did not have proper struct
device references (therefore no adequate hierarchy either).

Thanks to the work of Andrew, we now have proper MDIO, SPI, GPIO, I2C,
PCI, platform and drivers that allow us to register with DSA as a
specialized kind of device (so we are now finally using the right Linux
Device Driver model). What we still need though, in order to our switch
to the networking stack is a reference to the master/host network device
since we mangle packets in and out of it.

> 
>> @@ -495,7 +477,7 @@ static struct net_device *dev_to_net_device(struct device *dev)
>>  {
>>  	struct device *d;
>>  
>> -	d = dev_find_class(dev, "net");
>> +	d = device_find_class(dev, "net");
>>  	if (d != NULL) {
>>  		struct net_device *nd;
> 
> Again, huh?  What is the device heirachy here that is so odd that this
> type of function is needed?

An Ethernet switch managed by DSA needs to have one ore more references
to a host/CPU/management network interface, this is what this struct
device reference is here for.
--
Florian

Re: [PATCH net-next v3 05/10] drivers: base: Add device_find_class()

[Florian Fainelli]

On 01/15/2017 03:04 AM, Greg KH wrote:
> On Sat, Jan 14, 2017 at 01:47:08PM -0800, Florian Fainelli wrote:
>> Add a helper function to lookup a device reference given a class name.
>> This is a preliminary patch to remove adhoc code from net/dsa/dsa.c and
>> make it more generic.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  drivers/base/core.c    | 19 +++++++++++++++++++
>>  include/linux/device.h |  1 +
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index 020ea7f05520..3dd6047c10d8 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -2065,6 +2065,25 @@ struct device *device_find_child(struct device *parent, void *data,
>>  }
>>  EXPORT_SYMBOL_GPL(device_find_child);
>>  
>> +static int dev_is_class(struct device *dev, void *class)
>> +{
>> +	if (dev->class != NULL && !strcmp(dev->class->name, class))
>> +		return 1;
>> +
>> +	return 0;
>> +}
>> +
>> +struct device *device_find_class(struct device *parent, char *class)
> 
> Why are you using the char * for a class, and not just a pointer to
> "struct class"?  That seems to be the most logical one, no need to rely
> on string comparisons here.

A more reflective name of what that does would probably be
device_find_by_class_name() or something alike.

> 
> Also, what is this being used for?  You aren't trying to walk up the
> device heirachy to find a specific "type" of device, are you?  If so,
> ugh, I ranted about this in the past when the hyperv driver was trying
> to do such a thing...

What's a better way to do that though?

> 
>> +{
>> +	if (dev_is_class(parent, class)) {
>> +		get_device(parent);
>> +		return parent;
>> +	}
>> +
>> +	return device_find_child(parent, class, dev_is_class);
> 
> You are trying to find a peer device with the same parent that belongs
> to a specific class?

Correct, network devices, and MDIO bus devices usually (always?) set
dev.parent.

> 
> Again, what is this being used for?

See my other replies in patches 6, 7 and how it is used in patches 8 and
10 for instance.

> 
> And all exported driver core functions should have full kerneldoc
> information for them so that people know how to use them, and what the
> constraints are (see device_find_child() as an example.)  Please do that
> here as well because you are returning a pointer to a structure with the
> reference count incremented, callers need to know that.

Sure.
-- 
Florian

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Greg KH]

On Sun, Jan 15, 2017 at 09:27:22AM -0800, Florian Fainelli wrote:
> 
> 
> On 01/15/2017 03:06 AM, Greg KH wrote:
> > On Sat, Jan 14, 2017 at 01:47:09PM -0800, Florian Fainelli wrote:
> >> Now that the base device driver code provides an identical
> >> implementation of dev_find_class() utilize device_find_class() instead
> >> of our own version of it.
> >>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >> ---
> >>  net/dsa/dsa.c | 22 ++--------------------
> >>  1 file changed, 2 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> >> index 2306d1b87c83..77fa4c4f5828 100644
> >> --- a/net/dsa/dsa.c
> >> +++ b/net/dsa/dsa.c
> >> @@ -455,29 +455,11 @@ EXPORT_SYMBOL_GPL(dsa_switch_resume);
> >>  #endif
> >>  
> >>  /* platform driver init and cleanup *****************************************/
> >> -static int dev_is_class(struct device *dev, void *class)
> >> -{
> >> -	if (dev->class != NULL && !strcmp(dev->class->name, class))
> >> -		return 1;
> >> -
> >> -	return 0;
> >> -}
> >> -
> >> -static struct device *dev_find_class(struct device *parent, char *class)
> >> -{
> >> -	if (dev_is_class(parent, class)) {
> >> -		get_device(parent);
> >> -		return parent;
> >> -	}
> >> -
> >> -	return device_find_child(parent, class, dev_is_class);
> >> -}
> >> -
> >>  struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev)
> >>  {
> >>  	struct device *d;
> >>  
> >> -	d = dev_find_class(dev, "mdio_bus");
> >> +	d = device_find_class(dev, "mdio_bus");
> >>  	if (d != NULL) {
> >>  		struct mii_bus *bus;
> > 
> > You want a peer of your device on a specific class?  What is this for?
> 
> It's not a peer of our device, it's a separate device reference from the
> one looked up in the "net" class. In the classic, and now deprecated DSA
> device driver model, a "dsa" platform device would represent one or more
> Ethernet switches, connected via a MDIO bus (this reference above), and
> one Ethernet device (the CPU/host/management interface). This was
> completely violating the Linux device driver model and imposed
> limitations on what bus would be used, and we did not have proper struct
> device references (therefore no adequate hierarchy either).
> 
> Thanks to the work of Andrew, we now have proper MDIO, SPI, GPIO, I2C,
> PCI, platform and drivers that allow us to register with DSA as a
> specialized kind of device (so we are now finally using the right Linux
> Device Driver model). What we still need though, in order to our switch
> to the networking stack is a reference to the master/host network device
> since we mangle packets in and out of it.

Ok, but where in the tree are you trying to find this other device?  Are
you just going to randomly find any device that happens to be of the
specific class type?  That seems really fragile and broken :(

You should NEVER have to walk the device tree to find stuff.  Why can't
you have a pointer to the device you need to talk to some other way?

What exactly is the relationship between these devices (a ascii-art tree
or sysfs tree output might be nice) so I can try to understand what is
going on here.

thanks,

greg k-h

Re: [PATCH net-next v3 00/10] net: dsa: Support for pdata in dsa2

[Florian Fainelli]

On 01/15/2017 03:08 AM, Greg KH wrote:
> On Sat, Jan 14, 2017 at 01:47:03PM -0800, Florian Fainelli wrote:
>> Hi all,
>>
>> This is not exactly new, and was sent before, although back then, I did not
>> have an user of the pre-declared MDIO board information, but now we do. Note
>> that I have additional changes queued up to have b53 register platform data for
>> MIPS bcm47xx and bcm63xx.
>>
>> Yes I know that we should have the Orion platforms eventually be converted to
>> Device Tree, but until that happens, I don't want any remaining users of the
>> old "dsa" platform device (hence the previous DTS submissions for ARM/mvebu)
>> and, there will be platforms out there that most likely won't never see DT
>> coming their way (BCM47xx is almost 100% sure, BCM63xx maybe not in a distant
>> future).
>>
>> We would probably want the whole series to be merged via David Miller's tree
>> to simplify things.
>>
>> Greg, can you Ack/Nack patch 5 since it touched the core LDD?
> 
> I've NAKed them for now, you need to describe what you are trying to do
> here, as it doesn't make any sense to me at the moment.

For one, this is moving *existing* code from net/dsa/dsa.c part into the
device core for device_find_class() and part into the network device
core for dev_to_net_device(). Patch 8 is where this actually gets used.
See my individual replies for more details.

Even though the existing code is there in net/dsa/dsa.c, at this point,
and for the sake of getting these patches merged via David, I can
probably just keep it where it is (like what patch series v1 did) and
just namespace it with dsa_. Later on, if this is deemed valuable to
other parts of the kernel, I can try to relocate it to the device core,
does that sound acceptable?
-- 
Florian

Re: [PATCH net-next v3 07/10] net: Relocate dev_to_net_device() into core

[Greg KH]

On Sun, Jan 15, 2017 at 09:20:06AM -0800, Florian Fainelli wrote:
> 
> 
> On 01/15/2017 03:07 AM, Greg KH wrote:
> > On Sat, Jan 14, 2017 at 01:47:10PM -0800, Florian Fainelli wrote:
> >> dev_to_net_device() is moved from net/dsa/dsa.c to net/core/dev.c since
> >> it going to be used by net/dsa/dsa2.c and the namespace of the function
> >> justifies making it available to other users potentially.
> >>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >> ---
> >>  include/linux/netdevice.h |  2 ++
> >>  net/core/dev.c            | 19 +++++++++++++++++++
> >>  net/dsa/dsa.c             | 18 ------------------
> >>  3 files changed, 21 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> >> index 97ae0ac513ee..6d021c37b774 100644
> >> --- a/include/linux/netdevice.h
> >> +++ b/include/linux/netdevice.h
> >> @@ -4390,4 +4390,6 @@ do {								\
> >>  #define PTYPE_HASH_SIZE	(16)
> >>  #define PTYPE_HASH_MASK	(PTYPE_HASH_SIZE - 1)
> >>  
> >> +struct net_device *dev_to_net_device(struct device *dev);
> >> +
> >>  #endif	/* _LINUX_NETDEVICE_H */
> >> diff --git a/net/core/dev.c b/net/core/dev.c
> >> index ad5959e56116..7547e2ccc06b 100644
> >> --- a/net/core/dev.c
> >> +++ b/net/core/dev.c
> >> @@ -8128,6 +8128,25 @@ const char *netdev_drivername(const struct net_device *dev)
> >>  	return empty;
> >>  }
> >>  
> >> +struct net_device *dev_to_net_device(struct device *dev)
> >> +{
> >> +	struct device *d;
> >> +
> >> +	d = device_find_class(dev, "net");
> >> +	if (d) {
> >> +		struct net_device *nd;
> >> +
> >> +		nd = to_net_dev(d);
> >> +		dev_hold(nd);
> >> +		put_device(d);
> >> +
> >> +		return nd;
> >> +	}
> >> +
> >> +	return NULL;
> >> +}
> >> +EXPORT_SYMBOL_GPL(dev_to_net_device);
> > 
> > This really isn't just a "struct device to net device cast" type
> > function, (otherwise a simple container_of() would work).  You are
> > walking the device tree and assuming it is in a specific order so that
> > this function works.  You better document the hell out of this,
> > otherwise people are going to try to use this and get very confused,
> > very quickly...
> 
> Fair enough. Does that make it clearer how the device_find_class() is
> used though? Maybe device_find_class() should be named
> device_find_by_class_name() instead?

Better, but you are just poking around randomly in the device tree and
"hoping" you get it right.  What happens if devices move around?  You
are assuming some sort of heirachy here that I don't understand at
all...

thanks,

greg k-h

Re: [PATCH net-next v3 00/10] net: dsa: Support for pdata in dsa2

[Greg KH]

On Sun, Jan 15, 2017 at 09:40:24AM -0800, Florian Fainelli wrote:
> On 01/15/2017 03:08 AM, Greg KH wrote:
> > On Sat, Jan 14, 2017 at 01:47:03PM -0800, Florian Fainelli wrote:
> >> Hi all,
> >>
> >> This is not exactly new, and was sent before, although back then, I did not
> >> have an user of the pre-declared MDIO board information, but now we do. Note
> >> that I have additional changes queued up to have b53 register platform data for
> >> MIPS bcm47xx and bcm63xx.
> >>
> >> Yes I know that we should have the Orion platforms eventually be converted to
> >> Device Tree, but until that happens, I don't want any remaining users of the
> >> old "dsa" platform device (hence the previous DTS submissions for ARM/mvebu)
> >> and, there will be platforms out there that most likely won't never see DT
> >> coming their way (BCM47xx is almost 100% sure, BCM63xx maybe not in a distant
> >> future).
> >>
> >> We would probably want the whole series to be merged via David Miller's tree
> >> to simplify things.
> >>
> >> Greg, can you Ack/Nack patch 5 since it touched the core LDD?
> > 
> > I've NAKed them for now, you need to describe what you are trying to do
> > here, as it doesn't make any sense to me at the moment.
> 
> For one, this is moving *existing* code from net/dsa/dsa.c part into the
> device core for device_find_class() and part into the network device
> core for dev_to_net_device(). Patch 8 is where this actually gets used.
> See my individual replies for more details.
> 
> Even though the existing code is there in net/dsa/dsa.c, at this point,
> and for the sake of getting these patches merged via David, I can
> probably just keep it where it is (like what patch series v1 did) and
> just namespace it with dsa_. Later on, if this is deemed valuable to
> other parts of the kernel, I can try to relocate it to the device core,
> does that sound acceptable?

Nope!

I really want to try to understand what you all are doing with the
device tree that you feel that blindly walking it actually comes up with
a valid result.

See my other email about wanting to see a tree, we can take it from that
thread to try to consolidate all of these different ones.

And sorry, I know you are just trying to move code around, but this
isn't the first time this has come up, and I think it needs to be
resolved properly.

thanks,

greg k-h

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Florian Fainelli]

On 01/15/2017 09:39 AM, Greg KH wrote:
> On Sun, Jan 15, 2017 at 09:27:22AM -0800, Florian Fainelli wrote:
>>
>>
>> On 01/15/2017 03:06 AM, Greg KH wrote:
>>> On Sat, Jan 14, 2017 at 01:47:09PM -0800, Florian Fainelli wrote:
>>>> Now that the base device driver code provides an identical
>>>> implementation of dev_find_class() utilize device_find_class() instead
>>>> of our own version of it.
>>>>
>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>>> ---
>>>>  net/dsa/dsa.c | 22 ++--------------------
>>>>  1 file changed, 2 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
>>>> index 2306d1b87c83..77fa4c4f5828 100644
>>>> --- a/net/dsa/dsa.c
>>>> +++ b/net/dsa/dsa.c
>>>> @@ -455,29 +455,11 @@ EXPORT_SYMBOL_GPL(dsa_switch_resume);
>>>>  #endif
>>>>  
>>>>  /* platform driver init and cleanup *****************************************/
>>>> -static int dev_is_class(struct device *dev, void *class)
>>>> -{
>>>> -	if (dev->class != NULL && !strcmp(dev->class->name, class))
>>>> -		return 1;
>>>> -
>>>> -	return 0;
>>>> -}
>>>> -
>>>> -static struct device *dev_find_class(struct device *parent, char *class)
>>>> -{
>>>> -	if (dev_is_class(parent, class)) {
>>>> -		get_device(parent);
>>>> -		return parent;
>>>> -	}
>>>> -
>>>> -	return device_find_child(parent, class, dev_is_class);
>>>> -}
>>>> -
>>>>  struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev)
>>>>  {
>>>>  	struct device *d;
>>>>  
>>>> -	d = dev_find_class(dev, "mdio_bus");
>>>> +	d = device_find_class(dev, "mdio_bus");
>>>>  	if (d != NULL) {
>>>>  		struct mii_bus *bus;
>>>
>>> You want a peer of your device on a specific class?  What is this for?
>>
>> It's not a peer of our device, it's a separate device reference from the
>> one looked up in the "net" class. In the classic, and now deprecated DSA
>> device driver model, a "dsa" platform device would represent one or more
>> Ethernet switches, connected via a MDIO bus (this reference above), and
>> one Ethernet device (the CPU/host/management interface). This was
>> completely violating the Linux device driver model and imposed
>> limitations on what bus would be used, and we did not have proper struct
>> device references (therefore no adequate hierarchy either).
>>
>> Thanks to the work of Andrew, we now have proper MDIO, SPI, GPIO, I2C,
>> PCI, platform and drivers that allow us to register with DSA as a
>> specialized kind of device (so we are now finally using the right Linux
>> Device Driver model). What we still need though, in order to our switch
>> to the networking stack is a reference to the master/host network device
>> since we mangle packets in and out of it.
> 
> Ok, but where in the tree are you trying to find this other device?  Are
> you just going to randomly find any device that happens to be of the
> specific class type?  That seems really fragile and broken :(

The search is not really random, since the platform data for the DSA
switch we want to register gives us a reference to a device registered
via any bus type, and we know (by contract it has to) that it has to
provide a network device of some kind, we do a specific search in the
"net" class for that purpose. I agree this is not great.

> 
> You should NEVER have to walk the device tree to find stuff.  Why can't
> you have a pointer to the device you need to talk to some other way?

We do, I think the existing code just tries to be extra careful here and
make sure that the device reference we got is actually part of the "net"
class, indicating that the reference passed is indeed pointing to a
network device, and not a random device.

> 
> What exactly is the relationship between these devices (a ascii-art tree
> or sysfs tree output might be nice) so I can try to understand what is
> going on here.

OK, let me try and let's take the case of only one Ethernet switch in
the system for all simplicity:

- switch device drivers get probed by any kind of bus allocate and
register a dsa_switch with dsa_register_switch()

- this switch device has platform data (struct dsa_chip_data) that
describes its port layout (number of ports mostly) and has one struct
device references to which Ethernet network interface these ports connect to

- a dsa_switch_tree is created, this struct dsa_switch is attached to
the dsa_switch_tree at position 0 in the tree, we invoke a bunch of
switch driver operations

- we conclude with attaching this dsa_switch_tree to the network device
we looked up and assigning the tree to the dsa_ptr member

Documentation/networking/dsa/dsa.txt has some ascii art drawing that
sort of capture what is going on.
-- 
Florian

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Andrew Lunn]

> > What exactly is the relationship between these devices (a ascii-art tree
> > or sysfs tree output might be nice) so I can try to understand what is
> > going on here.

Hi Greg, Florian

A few diagrams and trees which might help understand what is going on.

The first diagram comes from the 2008 patch which added all this code:

            +-----------+       +-----------+
            |           | RGMII |           |
            |           +-------+           +------ 1000baseT MDI ("WAN")
            |           |       |  6-port   +------ 1000baseT MDI ("LAN1")
            |    CPU    |       |  ethernet +------ 1000baseT MDI ("LAN2")
            |           |MIImgmt|  switch   +------ 1000baseT MDI ("LAN3")
            |           +-------+  w/5 PHYs +------ 1000baseT MDI ("LAN4")
            |           |       |           |
            +-----------+       +-----------+

We have an ethernet switch and a host CPU. The switch is connected to
the CPU in two different ways. RGMII allows us to get Ethernet frames
from the CPU into the switch. MIImgmt, is the management bus normally
used for Ethernet PHYs, but Marvell switches also use it for Managing
switches.

The diagram above is the simplest setup. You can have multiple
Ethernet switches, connected together via switch ports. Each switch
has its own MIImgmt connect to the CPU, but there is only one RGMII
link.

When this code was designed back in 2008, it was decided to represent
this is a platform device, and it has a platform_data, which i have
slightly edited to keep it simple:

struct dsa_platform_data {
        /*
         * Reference to a Linux network interface that connects
         * to the root switch chip of the tree.
         */
        struct device   *netdev;

        /*
         * Info structs describing each of the switch chips
         * connected via this network interface.
         */
        int             nr_chips;
        struct dsa_chip_data    *chip;
};

This netdev is the CPU side of the RGMII interface.

Each switch has a dsa_chip_data, again edited:

struct dsa_chip_data {
        /*
         * How to access the switch configuration registers.
         */
        struct device   *host_dev;
        int             sw_addr;
...
}

The host_dev is the CPU side of the MIImgmt, and we have the address
the switch is using on the bus.

During probe of this platform device, we need to get from the
struct device *netdev to a struct net_device *dev.

So the code looks in the device net class to find the device

|   |   |   |-- f1074000.ethernet
|   |   |   |   |-- deferred_probe
|   |   |   |   |-- driver -> ../../../../../bus/platform/drivers/mvneta
|   |   |   |   |-- driver_override
|   |   |   |   |-- modalias
|   |   |   |   |-- net
|   |   |   |   |   `-- eth1
|   |   |   |   |       |-- addr_assign_type
|   |   |   |   |       |-- address
|   |   |   |   |       |-- addr_len
|   |   |   |   |       |-- broadcast
|   |   |   |   |       |-- carrier
|   |   |   |   |       |-- carrier_changes
|   |   |   |   |       |-- deferred_probe
|   |   |   |   |       |-- device -> ../../../f1074000.ethernet

and then use container_of() to get the net_device.

Similarly, the code needs to get from struct device *host_dev to a struct mii_bus *.

|   |   |   |-- f1072004.mdio
|   |   |   |   |-- deferred_probe
|   |   |   |   |-- driver -> ../../../../../bus/platform/drivers/orion-mdio
|   |   |   |   |-- driver_override
|   |   |   |   |-- mdio_bus
|   |   |   |   |   `-- f1072004.mdio-mi
|   |   |   |   |       |-- deferred_probe
|   |   |   |   |       |-- device -> ../../../f1072004.mdio

    Andrew

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Florian Fainelli]

On 01/15/2017 11:16 AM, Andrew Lunn wrote:
>>> What exactly is the relationship between these devices (a ascii-art tree
>>> or sysfs tree output might be nice) so I can try to understand what is
>>> going on here.
> 
> Hi Greg, Florian
> 
> A few diagrams and trees which might help understand what is going on.
> 
> The first diagram comes from the 2008 patch which added all this code:
> 
>             +-----------+       +-----------+
>             |           | RGMII |           |
>             |           +-------+           +------ 1000baseT MDI ("WAN")
>             |           |       |  6-port   +------ 1000baseT MDI ("LAN1")
>             |    CPU    |       |  ethernet +------ 1000baseT MDI ("LAN2")
>             |           |MIImgmt|  switch   +------ 1000baseT MDI ("LAN3")
>             |           +-------+  w/5 PHYs +------ 1000baseT MDI ("LAN4")
>             |           |       |           |
>             +-----------+       +-----------+
> 
> We have an ethernet switch and a host CPU. The switch is connected to
> the CPU in two different ways. RGMII allows us to get Ethernet frames
> from the CPU into the switch. MIImgmt, is the management bus normally
> used for Ethernet PHYs, but Marvell switches also use it for Managing
> switches.
> 
> The diagram above is the simplest setup. You can have multiple
> Ethernet switches, connected together via switch ports. Each switch
> has its own MIImgmt connect to the CPU, but there is only one RGMII
> link.
> 
> When this code was designed back in 2008, it was decided to represent
> this is a platform device, and it has a platform_data, which i have
> slightly edited to keep it simple:
> 
> struct dsa_platform_data {
>         /*
>          * Reference to a Linux network interface that connects
>          * to the root switch chip of the tree.
>          */
>         struct device   *netdev;
> 
>         /*
>          * Info structs describing each of the switch chips
>          * connected via this network interface.
>          */
>         int             nr_chips;
>         struct dsa_chip_data    *chip;
> };
> 
> This netdev is the CPU side of the RGMII interface.
> 
> Each switch has a dsa_chip_data, again edited:
> 
> struct dsa_chip_data {
>         /*
>          * How to access the switch configuration registers.
>          */
>         struct device   *host_dev;
>         int             sw_addr;
> ...
> }
> 
> The host_dev is the CPU side of the MIImgmt, and we have the address
> the switch is using on the bus.
> 
> During probe of this platform device, we need to get from the
> struct device *netdev to a struct net_device *dev.
> 
> So the code looks in the device net class to find the device
> 
> |   |   |   |-- f1074000.ethernet
> |   |   |   |   |-- deferred_probe
> |   |   |   |   |-- driver -> ../../../../../bus/platform/drivers/mvneta
> |   |   |   |   |-- driver_override
> |   |   |   |   |-- modalias
> |   |   |   |   |-- net
> |   |   |   |   |   `-- eth1
> |   |   |   |   |       |-- addr_assign_type
> |   |   |   |   |       |-- address
> |   |   |   |   |       |-- addr_len
> |   |   |   |   |       |-- broadcast
> |   |   |   |   |       |-- carrier
> |   |   |   |   |       |-- carrier_changes
> |   |   |   |   |       |-- deferred_probe
> |   |   |   |   |       |-- device -> ../../../f1074000.ethernet
> 
> and then use container_of() to get the net_device.
> 
> Similarly, the code needs to get from struct device *host_dev to a struct mii_bus *.
> 
> |   |   |   |-- f1072004.mdio
> |   |   |   |   |-- deferred_probe
> |   |   |   |   |-- driver -> ../../../../../bus/platform/drivers/orion-mdio
> |   |   |   |   |-- driver_override
> |   |   |   |   |-- mdio_bus
> |   |   |   |   |   `-- f1072004.mdio-mi
> |   |   |   |   |       |-- deferred_probe
> |   |   |   |   |       |-- device -> ../../../f1072004.mdio
> 

Thanks Andrew! Greg, does that make it clearer how these devices
references are used, do you still think the way this is done is wrong,
too cautious, or valid?
-- 
Florian

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Greg KH]

On Mon, Jan 16, 2017 at 12:01:02PM -0800, Florian Fainelli wrote:
> On 01/15/2017 11:16 AM, Andrew Lunn wrote:
> >>> What exactly is the relationship between these devices (a ascii-art tree
> >>> or sysfs tree output might be nice) so I can try to understand what is
> >>> going on here.
> > 
> > Hi Greg, Florian
> > 
> > A few diagrams and trees which might help understand what is going on.
> > 
> > The first diagram comes from the 2008 patch which added all this code:
> > 
> >             +-----------+       +-----------+
> >             |           | RGMII |           |
> >             |           +-------+           +------ 1000baseT MDI ("WAN")
> >             |           |       |  6-port   +------ 1000baseT MDI ("LAN1")
> >             |    CPU    |       |  ethernet +------ 1000baseT MDI ("LAN2")
> >             |           |MIImgmt|  switch   +------ 1000baseT MDI ("LAN3")
> >             |           +-------+  w/5 PHYs +------ 1000baseT MDI ("LAN4")
> >             |           |       |           |
> >             +-----------+       +-----------+
> > 
> > We have an ethernet switch and a host CPU. The switch is connected to
> > the CPU in two different ways. RGMII allows us to get Ethernet frames
> > from the CPU into the switch. MIImgmt, is the management bus normally
> > used for Ethernet PHYs, but Marvell switches also use it for Managing
> > switches.
> > 
> > The diagram above is the simplest setup. You can have multiple
> > Ethernet switches, connected together via switch ports. Each switch
> > has its own MIImgmt connect to the CPU, but there is only one RGMII
> > link.
> > 
> > When this code was designed back in 2008, it was decided to represent
> > this is a platform device, and it has a platform_data, which i have
> > slightly edited to keep it simple:
> > 
> > struct dsa_platform_data {
> >         /*
> >          * Reference to a Linux network interface that connects
> >          * to the root switch chip of the tree.
> >          */
> >         struct device   *netdev;
> > 
> >         /*
> >          * Info structs describing each of the switch chips
> >          * connected via this network interface.
> >          */
> >         int             nr_chips;
> >         struct dsa_chip_data    *chip;
> > };
> > 
> > This netdev is the CPU side of the RGMII interface.
> > 
> > Each switch has a dsa_chip_data, again edited:
> > 
> > struct dsa_chip_data {
> >         /*
> >          * How to access the switch configuration registers.
> >          */
> >         struct device   *host_dev;
> >         int             sw_addr;
> > ...
> > }
> > 
> > The host_dev is the CPU side of the MIImgmt, and we have the address
> > the switch is using on the bus.
> > 
> > During probe of this platform device, we need to get from the
> > struct device *netdev to a struct net_device *dev.
> > 
> > So the code looks in the device net class to find the device
> > 
> > |   |   |   |-- f1074000.ethernet
> > |   |   |   |   |-- deferred_probe
> > |   |   |   |   |-- driver -> ../../../../../bus/platform/drivers/mvneta
> > |   |   |   |   |-- driver_override
> > |   |   |   |   |-- modalias
> > |   |   |   |   |-- net
> > |   |   |   |   |   `-- eth1
> > |   |   |   |   |       |-- addr_assign_type
> > |   |   |   |   |       |-- address
> > |   |   |   |   |       |-- addr_len
> > |   |   |   |   |       |-- broadcast
> > |   |   |   |   |       |-- carrier
> > |   |   |   |   |       |-- carrier_changes
> > |   |   |   |   |       |-- deferred_probe
> > |   |   |   |   |       |-- device -> ../../../f1074000.ethernet
> > 
> > and then use container_of() to get the net_device.
> > 
> > Similarly, the code needs to get from struct device *host_dev to a struct mii_bus *.
> > 
> > |   |   |   |-- f1072004.mdio
> > |   |   |   |   |-- deferred_probe
> > |   |   |   |   |-- driver -> ../../../../../bus/platform/drivers/orion-mdio
> > |   |   |   |   |-- driver_override
> > |   |   |   |   |-- mdio_bus
> > |   |   |   |   |   `-- f1072004.mdio-mi
> > |   |   |   |   |       |-- deferred_probe
> > |   |   |   |   |       |-- device -> ../../../f1072004.mdio
> > 
> 
> Thanks Andrew! Greg, does that make it clearer how these devices
> references are used, do you still think the way this is done is wrong,
> too cautious, or valid?

I'm still not sold on it, I think there is something odd here with your
use/assumptions of the driver model.  Give me a few days to catch up
with other stuff to respond back please...

thanks,

greg k-h

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Greg KH]

On Mon, Jan 16, 2017 at 12:01:02PM -0800, Florian Fainelli wrote:
> On 01/15/2017 11:16 AM, Andrew Lunn wrote:
> >>> What exactly is the relationship between these devices (a ascii-art tree
> >>> or sysfs tree output might be nice) so I can try to understand what is
> >>> going on here.
> > 
> > Hi Greg, Florian
> > 
> > A few diagrams and trees which might help understand what is going on.
> > 
> > The first diagram comes from the 2008 patch which added all this code:
> > 
> >             +-----------+       +-----------+
> >             |           | RGMII |           |
> >             |           +-------+           +------ 1000baseT MDI ("WAN")
> >             |           |       |  6-port   +------ 1000baseT MDI ("LAN1")
> >             |    CPU    |       |  ethernet +------ 1000baseT MDI ("LAN2")
> >             |           |MIImgmt|  switch   +------ 1000baseT MDI ("LAN3")
> >             |           +-------+  w/5 PHYs +------ 1000baseT MDI ("LAN4")
> >             |           |       |           |
> >             +-----------+       +-----------+
> > 
> > We have an ethernet switch and a host CPU. The switch is connected to
> > the CPU in two different ways. RGMII allows us to get Ethernet frames
> > from the CPU into the switch. MIImgmt, is the management bus normally
> > used for Ethernet PHYs, but Marvell switches also use it for Managing
> > switches.
> > 
> > The diagram above is the simplest setup. You can have multiple
> > Ethernet switches, connected together via switch ports. Each switch
> > has its own MIImgmt connect to the CPU, but there is only one RGMII
> > link.
> > 
> > When this code was designed back in 2008, it was decided to represent
> > this is a platform device, and it has a platform_data, which i have
> > slightly edited to keep it simple:
> > 
> > struct dsa_platform_data {
> >         /*
> >          * Reference to a Linux network interface that connects
> >          * to the root switch chip of the tree.
> >          */
> >         struct device   *netdev;

This I think is the oddest thing, why do you need to have the "root
switch" here?  You seem to have dropped the next value in this
structure:
	struct net_device *of_netdev;

Isn't that the "real" net_device you need/want to get to here?

Or, when you set netdev, can't you also point to the "real" net_device
you want to later get to?

> >         /*
> >          * Info structs describing each of the switch chips
> >          * connected via this network interface.
> >          */
> >         int             nr_chips;
> >         struct dsa_chip_data    *chip;
> > };
> > 
> > This netdev is the CPU side of the RGMII interface.
> > 
> > Each switch has a dsa_chip_data, again edited:
> > 
> > struct dsa_chip_data {
> >         /*
> >          * How to access the switch configuration registers.
> >          */
> >         struct device   *host_dev;
> >         int             sw_addr;
> > ...
> > }

If each switch only has one dsa_chip_data, can't you point directly from
that structure back to the dsa_platform_device?  Or is that what this
"host_dev" pointer is pointing to?

> > The host_dev is the CPU side of the MIImgmt, and we have the address
> > the switch is using on the bus.

"cpu side" means what?  What 'struct device' is this?  The host
controller?  The platform device?  Something else?

> > During probe of this platform device, we need to get from the
> > struct device *netdev to a struct net_device *dev.

Wait, what exactly is this "struct device *"?  Who created it?

And why are you using a platform device?  Shouldn't this be a custom
bus?  I know people like to abuse platform devices, is that the case
here, or is it really driven only by a device tree representation?

Shouldn't you have a bus for RGMII devices?  Is that the real problem
here, you don't have a representation for your RGMII "bus" with a
controller to bundle everything under (like a USB host controller, it
bridges from one bus to another).

> > So the code looks in the device net class to find the device
> > 
> > |   |   |   |-- f1074000.ethernet
> > |   |   |   |   |-- deferred_probe
> > |   |   |   |   |-- driver -> ../../../../../bus/platform/drivers/mvneta
> > |   |   |   |   |-- driver_override
> > |   |   |   |   |-- modalias
> > |   |   |   |   |-- net
> > |   |   |   |   |   `-- eth1
> > |   |   |   |   |       |-- addr_assign_type
> > |   |   |   |   |       |-- address
> > |   |   |   |   |       |-- addr_len
> > |   |   |   |   |       |-- broadcast
> > |   |   |   |   |       |-- carrier
> > |   |   |   |   |       |-- carrier_changes
> > |   |   |   |   |       |-- deferred_probe
> > |   |   |   |   |       |-- device -> ../../../f1074000.ethernet
> > 
> > and then use container_of() to get the net_device.

What 'struct device *' are you passing to container_of() here?  The one
representing eth1?  What call is this?  And where are you trying to go
from there, to a peer?  Why?

> > Similarly, the code needs to get from struct device *host_dev to a struct mii_bus *.
> > 
> > |   |   |   |-- f1072004.mdio
> > |   |   |   |   |-- deferred_probe
> > |   |   |   |   |-- driver -> ../../../../../bus/platform/drivers/orion-mdio
> > |   |   |   |   |-- driver_override
> > |   |   |   |   |-- mdio_bus
> > |   |   |   |   |   `-- f1072004.mdio-mi
> > |   |   |   |   |       |-- deferred_probe
> > |   |   |   |   |       |-- device -> ../../../f1072004.mdio

Ok, this looks like the "peer" you want to get to from eth1, you want
f1072004.mdio-mi, right?

If so, why is eth1 not below f1072004.mdio-mi in the heirachy already?

Why is it up attached to the parent device, making this a "flat"
heirachy?

That's what is confusing about your functions, you are just walking all
of the "child" devices of a specific struct device, trying to find the
first random one that happens to belong to a specific 'bus' because you
are related to it.

Which is wrong, eth1 should be a child of your bus device, that way you
"know" how to get to it (it's your parent!).

So, I'm still confused, and still think this is all wrong, but feel free
to prove me wrong about this :)

thanks,

greg k-h

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Andrew Lunn]

> > > struct dsa_platform_data {
> > >         /*
> > >          * Reference to a Linux network interface that connects
> > >          * to the root switch chip of the tree.
> > >          */
> > >         struct device   *netdev;
> 
> This I think is the oddest thing, why do you need to have the "root
> switch" here?  You seem to have dropped the next value in this
> structure:
> 	struct net_device *of_netdev;

We are implementing platform_data for devices which don't support
device tree. When using OF, we don't have any of these issues. We can
go straight to the device.

It is a bit convoluted, but look at
arch/arm/mach-orion5x/rd88f5181l-ge-setup.c. It defines the start of
the dsa_platform_data in that file. It then gets passed through
common.c: orion5x_eth_switch_init() to 
arch/arm/plat-orion/common.c:orion_ge00_switch_init() :

void __init orion_ge00_switch_init(struct dsa_platform_data *d)
{
        int i;

        d->netdev = &orion_ge00.dev;
        for (i = 0; i < d->nr_chips; i++)
                d->chip[i].host_dev = &orion_ge_mvmdio.dev;

        platform_device_register_data(NULL, "dsa", 0, d, sizeof(d));
}

Where we have

static struct platform_device orion_ge00 = {
        .name           = MV643XX_ETH_NAME,
        .id             = 0,
        .num_resources  = 1,
        .resource       = orion_ge00_resources,
        .dev            = {
                .coherent_dma_mask      = DMA_BIT_MASK(32),
        },
};

So this is the platform device for the Ethernet device. We cannot go
to the net_device, because it does not exist until this Ethernet
platform device is instantiated.

> Shouldn't you have a bus for RGMII devices?  Is that the real problem
> here, you don't have a representation for your RGMII "bus" with a
> controller to bundle everything under (like a USB host controller, it
> bridges from one bus to another).

RGMII is not a bus. It is a point to point link. Normally, it is
between the Ethernet MAC and the Ethernet PHY. But you can also have
it between an Ethernet MAC and another Ethernet MAC. I'm not sure
describing this is a bus would be practical. It would mean every
ethernet driver also becomes a bus driver! Every Ethernet PHY would
become a bus device. That is a huge change, for a few legacy boards
which are not getting converted to device tree.

> If so, why is eth1 not below f1072004.mdio-mi in the heirachy already?

See the initial diagram above. The switch has two parents. It hangs of
an MDIO bus, and you would like to make RGMII also a bus. Can the
device model handle that? I thought it was a tree, not a graph?

       Andrew

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Greg KH]

On Thu, Jan 19, 2017 at 03:53:15PM +0100, Andrew Lunn wrote:
> > > > struct dsa_platform_data {
> > > >         /*
> > > >          * Reference to a Linux network interface that connects
> > > >          * to the root switch chip of the tree.
> > > >          */
> > > >         struct device   *netdev;
> > 
> > This I think is the oddest thing, why do you need to have the "root
> > switch" here?  You seem to have dropped the next value in this
> > structure:
> > 	struct net_device *of_netdev;
> 
> We are implementing platform_data for devices which don't support
> device tree. When using OF, we don't have any of these issues. We can
> go straight to the device.
> 
> It is a bit convoluted, but look at
> arch/arm/mach-orion5x/rd88f5181l-ge-setup.c. It defines the start of
> the dsa_platform_data in that file. It then gets passed through
> common.c: orion5x_eth_switch_init() to 
> arch/arm/plat-orion/common.c:orion_ge00_switch_init() :
> 
> void __init orion_ge00_switch_init(struct dsa_platform_data *d)
> {
>         int i;
> 
>         d->netdev = &orion_ge00.dev;
>         for (i = 0; i < d->nr_chips; i++)
>                 d->chip[i].host_dev = &orion_ge_mvmdio.dev;
> 
>         platform_device_register_data(NULL, "dsa", 0, d, sizeof(d));
> }
> 
> Where we have
> 
> static struct platform_device orion_ge00 = {
>         .name           = MV643XX_ETH_NAME,
>         .id             = 0,
>         .num_resources  = 1,
>         .resource       = orion_ge00_resources,
>         .dev            = {
>                 .coherent_dma_mask      = DMA_BIT_MASK(32),
>         },
> };
> 
> So this is the platform device for the Ethernet device. We cannot go
> to the net_device, because it does not exist until this Ethernet
> platform device is instantiated.

Ok, fine, but why isn't the ethernet device a child of this platform
device?  Why is it floating around somewhere else?  You don't see that
happening for other devices.

> > Shouldn't you have a bus for RGMII devices?  Is that the real problem
> > here, you don't have a representation for your RGMII "bus" with a
> > controller to bundle everything under (like a USB host controller, it
> > bridges from one bus to another).
> 
> RGMII is not a bus. It is a point to point link.

That's fine, but you have multiple devices talking across it, so in the
kernel driver model "naming", it's a bus.  Anything can be a bus, it's
just a way to group together devices of the same type.

> Normally, it is
> between the Ethernet MAC and the Ethernet PHY. But you can also have
> it between an Ethernet MAC and another Ethernet MAC. I'm not sure
> describing this is a bus would be practical. It would mean every
> ethernet driver also becomes a bus driver!

Instead of a custom platform device driver, yes.  Is that a big deal?
How many do you have?

> Every Ethernet PHY would become a bus device. That is a huge change,
> for a few legacy boards which are not getting converted to device
> tree.

How many different drivers are we talking about here?

> > If so, why is eth1 not below f1072004.mdio-mi in the heirachy already?
> 
> See the initial diagram above. The switch has two parents. It hangs of
> an MDIO bus, and you would like to make RGMII also a bus. Can the
> device model handle that? I thought it was a tree, not a graph?

It is a tree, you are correct.  But right now you are picking and
choosing where you want to put that network device.  Why not put it over
on the mdio bus?  Or, like I mentioned, make it a custom bus where you
can properly show this relationship, not just in a generic "let's jump
to the parent and poke around randomly."

Again, it's that last sentance that I object the most to here.  You all
keep ignoring it for some reason...

thanks,

greg k-h

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Russell King - ARM Linux]

On Thu, Jan 19, 2017 at 05:30:13PM +0100, Greg KH wrote:
> On Thu, Jan 19, 2017 at 03:53:15PM +0100, Andrew Lunn wrote:
> > > > > struct dsa_platform_data {
> > > > >         /*
> > > > >          * Reference to a Linux network interface that connects
> > > > >          * to the root switch chip of the tree.
> > > > >          */
> > > > >         struct device   *netdev;
> > > 
> > > This I think is the oddest thing, why do you need to have the "root
> > > switch" here?  You seem to have dropped the next value in this
> > > structure:
> > > 	struct net_device *of_netdev;
> > 
> > We are implementing platform_data for devices which don't support
> > device tree. When using OF, we don't have any of these issues. We can
> > go straight to the device.
> > 
> > It is a bit convoluted, but look at
> > arch/arm/mach-orion5x/rd88f5181l-ge-setup.c. It defines the start of
> > the dsa_platform_data in that file. It then gets passed through
> > common.c: orion5x_eth_switch_init() to 
> > arch/arm/plat-orion/common.c:orion_ge00_switch_init() :
> > 
> > void __init orion_ge00_switch_init(struct dsa_platform_data *d)
> > {
> >         int i;
> > 
> >         d->netdev = &orion_ge00.dev;
> >         for (i = 0; i < d->nr_chips; i++)
> >                 d->chip[i].host_dev = &orion_ge_mvmdio.dev;
> > 
> >         platform_device_register_data(NULL, "dsa", 0, d, sizeof(d));
> > }
> > 
> > Where we have
> > 
> > static struct platform_device orion_ge00 = {
> >         .name           = MV643XX_ETH_NAME,
> >         .id             = 0,
> >         .num_resources  = 1,
> >         .resource       = orion_ge00_resources,
> >         .dev            = {
> >                 .coherent_dma_mask      = DMA_BIT_MASK(32),
> >         },
> > };
> > 
> > So this is the platform device for the Ethernet device. We cannot go
> > to the net_device, because it does not exist until this Ethernet
> > platform device is instantiated.
> 
> Ok, fine, but why isn't the ethernet device a child of this platform
> device?  Why is it floating around somewhere else?  You don't see that
> happening for other devices.

The ethernet device is not a child of the DSA device.  I'm going to
send a mail I've had queued up for a few hours redoing Andrew's
ASCII art, because I think that's what's causing the confusion here,
provided I haven't discarded it.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Russell King - ARM Linux]

(This is mainly for Greg's benefit to help him understand the issue.)

I think the diagram you gave initially made this confusing, as it
talks about a CPU(sic) producing the "RGMII" and "MII-MGMT".

Let's instead show a better representation that hopefully helps Greg
understand networking. :)


  CPU
System <-B->  Ethernet controller <-P-> } PHY <---> network cable
                                        } - - - - - - - or - - - - - - -
                  MDIO bus -------M---> } Switch <-P-> PHYs <--> network
                                              `----M----^        cables

'B' can be an on-SoC bus or something like PCI.

'P' are the high-speed connectivity between the ethernet controller and
PHY which carries the packet data.  It has no addressing, it's a point
to point link.  RGMII is just one wiring example, there are many
different interfaces there (SGMII, AUI, XAUI, XGMII to name a few.)

'M' are the MDIO bus, which is the bus by which ethernet PHYs and
switches can be identified and controlled.

The MDIO bus has a bus_type, has host drivers which are sometimes
part of the ethernet controller, but can also be stand-alone devices
shared between multiple ethernet controllers.

PHYs are a kind of MDIO device which are members of the MDIO bus
type.  Each PHY (and switch) has a numerical address, and identifying
numbers within its register set which identifies the manufacturer
and device type.  We have device_driver objects for these.

Expanding the above diagram to make it (hopefully) even clearer,
we can have this classic setup:

  CPU
System <-B-> Ethernet controller <-P-> PHY <---> network cable
                 MDIO bus -------M------^

Or, in the case of two DSA switches attached to an Ethernet controller:

                                     |~~~~~~~~|
System <-B-> Ethernet controller <-P-> Switch <-P-> PHY1 <--> network cable
                 MDIO bus ----+--M--->   1    <-P-> PHY2 <--> network cable
                              |      |        ...    |
                              |      |        <-P-> PHYn <--> network cable
                              |      |....^...|      |
                              |           |  `---M---'
                              |           P
                              |           |
                              |      |~~~~v~~~|
                              `------> Switch <-P-> PHY1 <--> network cable
                                     |   2    ...    |
                                     |        <-P-> PHYn <--> network cable
                                     |........|      |
                                             `---M---'

The problem that the DSA guys are trying to deal with is how to
represent the link between the DSA switches (which are devices
sitting off their controlling bus - the MDIO bus) and the ethernet
controller associated with that collection of devices, be it a
switch or PHY.

Merely changing the parent/child relationships to try and solve
one issue just creates exactly the same problem elsewhere.

So, I hope with these diagrams, you can see that trying to make
the ethernet controller a child device of the DSA switches
means that (eg) it's no longer a PCI device, which is rather
absurd, especially when considering that what happens to the
right of the ethernet controller in the diagrams above is
normally external chips to the SoC or ethernet device.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Florian Fainelli]

On 01/19/2017 08:51 AM, Russell King - ARM Linux wrote:
> (This is mainly for Greg's benefit to help him understand the issue.)
> 
> I think the diagram you gave initially made this confusing, as it
> talks about a CPU(sic) producing the "RGMII" and "MII-MGMT".
> 
> Let's instead show a better representation that hopefully helps Greg
> understand networking. :)
> 
> 
>   CPU
> System <-B->  Ethernet controller <-P-> } PHY <---> network cable
>                                         } - - - - - - - or - - - - - - -
>                   MDIO bus -------M---> } Switch <-P-> PHYs <--> network
>                                               `----M----^        cables
> 
> 'B' can be an on-SoC bus or something like PCI.
> 
> 'P' are the high-speed connectivity between the ethernet controller and
> PHY which carries the packet data.  It has no addressing, it's a point
> to point link.  RGMII is just one wiring example, there are many
> different interfaces there (SGMII, AUI, XAUI, XGMII to name a few.)
> 
> 'M' are the MDIO bus, which is the bus by which ethernet PHYs and
> switches can be identified and controlled.

So MDIO is one possible bus type to connect an Ethernet switch (and
PHYs) to a the System, but is not necessarily the only one. You have
existing devices out there with on-chip integrated switches (platform),
SPI/GPIO/I2C/PCI(e).

> 
> The MDIO bus has a bus_type, has host drivers which are sometimes
> part of the ethernet controller, but can also be stand-alone devices
> shared between multiple ethernet controllers.
> 
> PHYs are a kind of MDIO device which are members of the MDIO bus
> type.  Each PHY (and switch) has a numerical address, and identifying
> numbers within its register set which identifies the manufacturer
> and device type.  We have device_driver objects for these.
> 
> Expanding the above diagram to make it (hopefully) even clearer,
> we can have this classic setup:
> 
>   CPU
> System <-B-> Ethernet controller <-P-> PHY <---> network cable
>                  MDIO bus -------M------^
> 
> Or, in the case of two DSA switches attached to an Ethernet controller:
> 
>                                      |~~~~~~~~|
> System <-B-> Ethernet controller <-P-> Switch <-P-> PHY1 <--> network cable
>                  MDIO bus ----+--M--->   1    <-P-> PHY2 <--> network cable
>                               |      |        ...    |
>                               |      |        <-P-> PHYn <--> network cable
>                               |      |....^...|      |
>                               |           |  `---M---'
>                               |           P
>                               |           |
>                               |      |~~~~v~~~|
>                               `------> Switch <-P-> PHY1 <--> network cable
>                                      |   2    ...    |
>                                      |        <-P-> PHYn <--> network cable
>                                      |........|      |
>                                              `---M---'
> 
> The problem that the DSA guys are trying to deal with is how to
> represent the link between the DSA switches (which are devices
> sitting off their controlling bus - the MDIO bus) and the ethernet
> controller associated with that collection of devices, be it a
> switch or PHY.
> 
> Merely changing the parent/child relationships to try and solve
> one issue just creates exactly the same problem elsewhere.

Exactly!

> 
> So, I hope with these diagrams, you can see that trying to make
> the ethernet controller a child device of the DSA switches
> means that (eg) it's no longer a PCI device, which is rather
> absurd, especially when considering that what happens to the
> right of the ethernet controller in the diagrams above is
> normally external chips to the SoC or ethernet device.
> 

Indeed.

Back to the actual code that triggered this discussion, the whole
purpose is just a safeguard. Given a device reference, we can assume
that it is indeed the backing device for a net_device, and we could do a
to_net_device() right away (and crash if someone did not write correct
platform_data structures), or, by walking the device tree (the device
driver model one) we can make sure it does belong in the proper class
and this is indeed what we think it is.

HTH
-- 
Florian

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Florian Fainelli]

On 01/19/2017 10:12 AM, Florian Fainelli wrote:
> 
> Back to the actual code that triggered this discussion, the whole
> purpose is just a safeguard. Given a device reference, we can assume
> that it is indeed the backing device for a net_device, and we could do a
> to_net_device() right away (and crash if someone did not write correct
> platform_data structures), or, by walking the device tree (the device
> driver model one) we can make sure it does belong in the proper class
> and this is indeed what we think it is.

Greg, did Russell's explanation clarify things, or do you still think
this is completely bogus and we need to re design the whole thing?

Just asking so I can try to resubmit just the preparatory parts or just
the whole thing.

Thank you
-- 
Florian

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Greg KH]

On Tue, Jan 24, 2017 at 10:59:15AM -0800, Florian Fainelli wrote:
> On 01/19/2017 10:12 AM, Florian Fainelli wrote:
> > 
> > Back to the actual code that triggered this discussion, the whole
> > purpose is just a safeguard. Given a device reference, we can assume
> > that it is indeed the backing device for a net_device, and we could do a
> > to_net_device() right away (and crash if someone did not write correct
> > platform_data structures), or, by walking the device tree (the device
> > driver model one) we can make sure it does belong in the proper class
> > and this is indeed what we think it is.
> 
> Greg, did Russell's explanation clarify things, or do you still think
> this is completely bogus and we need to re design the whole thing?
> 
> Just asking so I can try to resubmit just the preparatory parts or just
> the whole thing.

Sorry, I haven't gotten back to this, it's lower on my list.  Should try
to get to it tomorrow...

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Florian Fainelli]

On 01/25/2017 01:25 PM, Greg KH wrote:
> On Tue, Jan 24, 2017 at 10:59:15AM -0800, Florian Fainelli wrote:
>> On 01/19/2017 10:12 AM, Florian Fainelli wrote:
>>>
>>> Back to the actual code that triggered this discussion, the whole
>>> purpose is just a safeguard. Given a device reference, we can assume
>>> that it is indeed the backing device for a net_device, and we could do a
>>> to_net_device() right away (and crash if someone did not write correct
>>> platform_data structures), or, by walking the device tree (the device
>>> driver model one) we can make sure it does belong in the proper class
>>> and this is indeed what we think it is.
>>
>> Greg, did Russell's explanation clarify things, or do you still think
>> this is completely bogus and we need to re design the whole thing?
>>
>> Just asking so I can try to resubmit just the preparatory parts or just
>> the whole thing.
> 
> Sorry, I haven't gotten back to this, it's lower on my list.  Should try
> to get to it tomorrow...

Greg, please give some feedback here, I can only produce new patches as
fast as I am given feedback, and I would really hate to miss the 4.11
merge window because we have been sleeping on this. If there is a need
to clarify things, I will be more than happy to try to provide information.

Thank you!
-- 
Florian

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Greg KH]

On Thu, Jan 19, 2017 at 04:51:55PM +0000, Russell King - ARM Linux wrote:
> (This is mainly for Greg's benefit to help him understand the issue.)
> 
> I think the diagram you gave initially made this confusing, as it
> talks about a CPU(sic) producing the "RGMII" and "MII-MGMT".
> 
> Let's instead show a better representation that hopefully helps Greg
> understand networking. :)
> 
> 
>   CPU
> System <-B->  Ethernet controller <-P-> } PHY <---> network cable
>                                         } - - - - - - - or - - - - - - -
>                   MDIO bus -------M---> } Switch <-P-> PHYs <--> network
>                                               `----M----^        cables
> 
> 'B' can be an on-SoC bus or something like PCI.
> 
> 'P' are the high-speed connectivity between the ethernet controller and
> PHY which carries the packet data.  It has no addressing, it's a point
> to point link.  RGMII is just one wiring example, there are many
> different interfaces there (SGMII, AUI, XAUI, XGMII to name a few.)
> 
> 'M' are the MDIO bus, which is the bus by which ethernet PHYs and
> switches can be identified and controlled.
> 
> The MDIO bus has a bus_type, has host drivers which are sometimes
> part of the ethernet controller, but can also be stand-alone devices
> shared between multiple ethernet controllers.
> 
> PHYs are a kind of MDIO device which are members of the MDIO bus
> type.  Each PHY (and switch) has a numerical address, and identifying
> numbers within its register set which identifies the manufacturer
> and device type.  We have device_driver objects for these.
> 
> Expanding the above diagram to make it (hopefully) even clearer,
> we can have this classic setup:
> 
>   CPU
> System <-B-> Ethernet controller <-P-> PHY <---> network cable
>                  MDIO bus -------M------^
> 
> Or, in the case of two DSA switches attached to an Ethernet controller:
> 
>                                      |~~~~~~~~|
> System <-B-> Ethernet controller <-P-> Switch <-P-> PHY1 <--> network cable
>                  MDIO bus ----+--M--->   1    <-P-> PHY2 <--> network cable
>                               |      |        ...    |
>                               |      |        <-P-> PHYn <--> network cable
>                               |      |....^...|      |
>                               |           |  `---M---'
>                               |           P
>                               |           |
>                               |      |~~~~v~~~|
>                               `------> Switch <-P-> PHY1 <--> network cable
>                                      |   2    ...    |
>                                      |        <-P-> PHYn <--> network cable
>                                      |........|      |
>                                              `---M---'
> 
> The problem that the DSA guys are trying to deal with is how to
> represent the link between the DSA switches (which are devices
> sitting off their controlling bus - the MDIO bus) and the ethernet
> controller associated with that collection of devices, be it a
> switch or PHY.

Why do they have to represent that link?  This is a driver that somehow
binds the two togther in some sort of "control plane"?

> Merely changing the parent/child relationships to try and solve
> one issue just creates exactly the same problem elsewhere.

Fair enough.

> So, I hope with these diagrams, you can see that trying to make
> the ethernet controller a child device of the DSA switches
> means that (eg) it's no longer a PCI device, which is rather
> absurd, especially when considering that what happens to the
> right of the ethernet controller in the diagrams above is
> normally external chips to the SoC or ethernet device.

Ok, thanks for the long explainations and diagrams.

_BUT_ my original point remains.  These new functions you all are trying
to get into the driver core, do NOT do what they say they are doing.
They are mucking around with a "known topology" and just happen to work
because the device you are trying to find shares a common parent with
yourself.

That is not what the function says it does, and as such, I do not want
that function in the driver core at all.

If you wish to keep it in your own subsystem, that's fine, but call it
what it really is:
	hack_to_find_peer_device_on_random_bus()
and pass in a _real_ pointer to a bus type.  Not some random string
please.

Or better yet, have the DSA code accept pointers to the two devices in
the first place, so it "knows" what to do here in a much better way.
Right now it is a bad hack.  You all can not argue that is not true.

thanks,

greg k-h

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Florian Fainelli]

On 02/10/2017 05:02 AM, Greg KH wrote:
> On Thu, Jan 19, 2017 at 04:51:55PM +0000, Russell King - ARM Linux wrote:
>> (This is mainly for Greg's benefit to help him understand the issue.)
>>
>> I think the diagram you gave initially made this confusing, as it
>> talks about a CPU(sic) producing the "RGMII" and "MII-MGMT".
>>
>> Let's instead show a better representation that hopefully helps Greg
>> understand networking. :)
>>
>>
>>   CPU
>> System <-B->  Ethernet controller <-P-> } PHY <---> network cable
>>                                         } - - - - - - - or - - - - - - -
>>                   MDIO bus -------M---> } Switch <-P-> PHYs <--> network
>>                                               `----M----^        cables
>>
>> 'B' can be an on-SoC bus or something like PCI.
>>
>> 'P' are the high-speed connectivity between the ethernet controller and
>> PHY which carries the packet data.  It has no addressing, it's a point
>> to point link.  RGMII is just one wiring example, there are many
>> different interfaces there (SGMII, AUI, XAUI, XGMII to name a few.)
>>
>> 'M' are the MDIO bus, which is the bus by which ethernet PHYs and
>> switches can be identified and controlled.
>>
>> The MDIO bus has a bus_type, has host drivers which are sometimes
>> part of the ethernet controller, but can also be stand-alone devices
>> shared between multiple ethernet controllers.
>>
>> PHYs are a kind of MDIO device which are members of the MDIO bus
>> type.  Each PHY (and switch) has a numerical address, and identifying
>> numbers within its register set which identifies the manufacturer
>> and device type.  We have device_driver objects for these.
>>
>> Expanding the above diagram to make it (hopefully) even clearer,
>> we can have this classic setup:
>>
>>   CPU
>> System <-B-> Ethernet controller <-P-> PHY <---> network cable
>>                  MDIO bus -------M------^
>>
>> Or, in the case of two DSA switches attached to an Ethernet controller:
>>
>>                                      |~~~~~~~~|
>> System <-B-> Ethernet controller <-P-> Switch <-P-> PHY1 <--> network cable
>>                  MDIO bus ----+--M--->   1    <-P-> PHY2 <--> network cable
>>                               |      |        ...    |
>>                               |      |        <-P-> PHYn <--> network cable
>>                               |      |....^...|      |
>>                               |           |  `---M---'
>>                               |           P
>>                               |           |
>>                               |      |~~~~v~~~|
>>                               `------> Switch <-P-> PHY1 <--> network cable
>>                                      |   2    ...    |
>>                                      |        <-P-> PHYn <--> network cable
>>                                      |........|      |
>>                                              `---M---'
>>
>> The problem that the DSA guys are trying to deal with is how to
>> represent the link between the DSA switches (which are devices
>> sitting off their controlling bus - the MDIO bus) and the ethernet
>> controller associated with that collection of devices, be it a
>> switch or PHY.
> 
> Why do they have to represent that link?  This is a driver that somehow
> binds the two togther in some sort of "control plane"?

We have to represent that link because the CPU/host/management Ethernet
MAC is physically connected to the CPU/management port of the switch. It
does indeed participate in establishing the control plane.

The basic idea of DSA is that the switch inserts vendor tags to indicate
why the packet is sent towards the CPU in the first place: flooding,
management, copy etc along with information as to which
originating/destination port(s) this packet comes/goes from/to. On top
of that, we demultiplex that tag to deliver normal Ethernet frames to
per-port network devices (virtual network devices).

If we did leave the switch in an unmanaged mode and not logically
attached to an Ethernet MAC for management, we'd lose all that
information (we could use per-port VLANs to re-create it, but it would
be inferior to what a switch with proprietary tags can do)

Code in net/dsa/dsa2.c that binds the two (switch and Ethernet MAC)
together is not strictly a driver, it just is resident in memory and
waits for dsa_register_switch() to be called until it tries to do this
binding.

> 
>> Merely changing the parent/child relationships to try and solve
>> one issue just creates exactly the same problem elsewhere.
> 
> Fair enough.
> 
>> So, I hope with these diagrams, you can see that trying to make
>> the ethernet controller a child device of the DSA switches
>> means that (eg) it's no longer a PCI device, which is rather
>> absurd, especially when considering that what happens to the
>> right of the ethernet controller in the diagrams above is
>> normally external chips to the SoC or ethernet device.
> 
> Ok, thanks for the long explainations and diagrams.
> 
> _BUT_ my original point remains.  These new functions you all are trying
> to get into the driver core, do NOT do what they say they are doing.
> They are mucking around with a "known topology" and just happen to work
> because the device you are trying to find shares a common parent with
> yourself.
> 
> That is not what the function says it does, and as such, I do not want
> that function in the driver core at all.
> 
> If you wish to keep it in your own subsystem, that's fine, but call it
> what it really is:
> 	hack_to_find_peer_device_on_random_bus()
> and pass in a _real_ pointer to a bus type.  Not some random string
> please.

Yes, David has accepted that and we did make it prefixed with dsa_*.
That does not mean it should stay forever though if we can work on
something better designed.

> 
> Or better yet, have the DSA code accept pointers to the two devices in
> the first place, so it "knows" what to do here in a much better way.

Well, that is really what is being done here, the "inputs" to the code
in net/dsa/dsa2.c are opaque struct device references that we
verify/proof by making sure they belong in the class of device we expect
them to be.

NB: technically it's just one device reference: Ethernet MAC device,
because the other one is implied by the device driver registering the
switch with dsa_register_switch() (MDIO, SPI, I2C, PCI etc..).

> Right now it is a bad hack.  You all can not argue that is not true.

It's hard to argue on an qualifier for that design when we can't even
agree whether our understanding of these devices matches your
understanding after you read and digested our explanations :)

I entirely agree that the design has shortcomings, and in fact, it does
present some challenges that are not well solved right now, if you
unbind the Ethernet MAC driver, you leave a dangling DSA switch tree,
and good luck re-attaching it.

So maybe we should talk about it in person at $next conference, I will
be at ELC, how about you?

Thanks
-- 
Florian

Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()

[Greg KH]

On Fri, Feb 10, 2017 at 10:30:58AM -0800, Florian Fainelli wrote:
> On 02/10/2017 05:02 AM, Greg KH wrote:
> > On Thu, Jan 19, 2017 at 04:51:55PM +0000, Russell King - ARM Linux wrote:
> >> (This is mainly for Greg's benefit to help him understand the issue.)
> >>
> >> I think the diagram you gave initially made this confusing, as it
> >> talks about a CPU(sic) producing the "RGMII" and "MII-MGMT".
> >>
> >> Let's instead show a better representation that hopefully helps Greg
> >> understand networking. :)
> >>
> >>
> >>   CPU
> >> System <-B->  Ethernet controller <-P-> } PHY <---> network cable
> >>                                         } - - - - - - - or - - - - - - -
> >>                   MDIO bus -------M---> } Switch <-P-> PHYs <--> network
> >>                                               `----M----^        cables
> >>
> >> 'B' can be an on-SoC bus or something like PCI.
> >>
> >> 'P' are the high-speed connectivity between the ethernet controller and
> >> PHY which carries the packet data.  It has no addressing, it's a point
> >> to point link.  RGMII is just one wiring example, there are many
> >> different interfaces there (SGMII, AUI, XAUI, XGMII to name a few.)
> >>
> >> 'M' are the MDIO bus, which is the bus by which ethernet PHYs and
> >> switches can be identified and controlled.
> >>
> >> The MDIO bus has a bus_type, has host drivers which are sometimes
> >> part of the ethernet controller, but can also be stand-alone devices
> >> shared between multiple ethernet controllers.
> >>
> >> PHYs are a kind of MDIO device which are members of the MDIO bus
> >> type.  Each PHY (and switch) has a numerical address, and identifying
> >> numbers within its register set which identifies the manufacturer
> >> and device type.  We have device_driver objects for these.
> >>
> >> Expanding the above diagram to make it (hopefully) even clearer,
> >> we can have this classic setup:
> >>
> >>   CPU
> >> System <-B-> Ethernet controller <-P-> PHY <---> network cable
> >>                  MDIO bus -------M------^
> >>
> >> Or, in the case of two DSA switches attached to an Ethernet controller:
> >>
> >>                                      |~~~~~~~~|
> >> System <-B-> Ethernet controller <-P-> Switch <-P-> PHY1 <--> network cable
> >>                  MDIO bus ----+--M--->   1    <-P-> PHY2 <--> network cable
> >>                               |      |        ...    |
> >>                               |      |        <-P-> PHYn <--> network cable
> >>                               |      |....^...|      |
> >>                               |           |  `---M---'
> >>                               |           P
> >>                               |           |
> >>                               |      |~~~~v~~~|
> >>                               `------> Switch <-P-> PHY1 <--> network cable
> >>                                      |   2    ...    |
> >>                                      |        <-P-> PHYn <--> network cable
> >>                                      |........|      |
> >>                                              `---M---'
> >>
> >> The problem that the DSA guys are trying to deal with is how to
> >> represent the link between the DSA switches (which are devices
> >> sitting off their controlling bus - the MDIO bus) and the ethernet
> >> controller associated with that collection of devices, be it a
> >> switch or PHY.
> > 
> > Why do they have to represent that link?  This is a driver that somehow
> > binds the two togther in some sort of "control plane"?
> 
> We have to represent that link because the CPU/host/management Ethernet
> MAC is physically connected to the CPU/management port of the switch. It
> does indeed participate in establishing the control plane.

But who uses that "link"?  What in userspace cares about it?  What in
the kernel cares?

> The basic idea of DSA is that the switch inserts vendor tags to indicate
> why the packet is sent towards the CPU in the first place: flooding,
> management, copy etc along with information as to which
> originating/destination port(s) this packet comes/goes from/to. On top
> of that, we demultiplex that tag to deliver normal Ethernet frames to
> per-port network devices (virtual network devices).
> 
> If we did leave the switch in an unmanaged mode and not logically
> attached to an Ethernet MAC for management, we'd lose all that
> information (we could use per-port VLANs to re-create it, but it would
> be inferior to what a switch with proprietary tags can do)
> 
> Code in net/dsa/dsa2.c that binds the two (switch and Ethernet MAC)
> together is not strictly a driver, it just is resident in memory and
> waits for dsa_register_switch() to be called until it tries to do this
> binding.

Ok, but when that "binding" happens, why do you need to show it in
sysfs?

> >> Merely changing the parent/child relationships to try and solve
> >> one issue just creates exactly the same problem elsewhere.
> > 
> > Fair enough.
> > 
> >> So, I hope with these diagrams, you can see that trying to make
> >> the ethernet controller a child device of the DSA switches
> >> means that (eg) it's no longer a PCI device, which is rather
> >> absurd, especially when considering that what happens to the
> >> right of the ethernet controller in the diagrams above is
> >> normally external chips to the SoC or ethernet device.
> > 
> > Ok, thanks for the long explainations and diagrams.
> > 
> > _BUT_ my original point remains.  These new functions you all are trying
> > to get into the driver core, do NOT do what they say they are doing.
> > They are mucking around with a "known topology" and just happen to work
> > because the device you are trying to find shares a common parent with
> > yourself.
> > 
> > That is not what the function says it does, and as such, I do not want
> > that function in the driver core at all.
> > 
> > If you wish to keep it in your own subsystem, that's fine, but call it
> > what it really is:
> > 	hack_to_find_peer_device_on_random_bus()
> > and pass in a _real_ pointer to a bus type.  Not some random string
> > please.
> 
> Yes, David has accepted that and we did make it prefixed with dsa_*.
> That does not mean it should stay forever though if we can work on
> something better designed.

Maybe, but really, this is a special case.

> > Or better yet, have the DSA code accept pointers to the two devices in
> > the first place, so it "knows" what to do here in a much better way.
> 
> Well, that is really what is being done here, the "inputs" to the code
> in net/dsa/dsa2.c are opaque struct device references that we
> verify/proof by making sure they belong in the class of device we expect
> them to be.

Why not take pointers to the class devices you expect in the first
place?  No need to take an "opaque" pointer, right?  Then nothing
special needs to be found anywhere else.

> NB: technically it's just one device reference: Ethernet MAC device,
> because the other one is implied by the device driver registering the
> switch with dsa_register_switch() (MDIO, SPI, I2C, PCI etc..).
> 
> > Right now it is a bad hack.  You all can not argue that is not true.
> 
> It's hard to argue on an qualifier for that design when we can't even
> agree whether our understanding of these devices matches your
> understanding after you read and digested our explanations :)

I think so, but again, you all don't seem to understand just why this is
such a bad hack of the driver model...

The main reason we don't have a "what type of device is this device
pointer" is that you are always supposed to "just know" what type it is,
because someone gave you the right type (i.e. the driver core, or
yourself).  So when you do random walks of the tree, and just expect
random strings to match up, it scares me.

> I entirely agree that the design has shortcomings, and in fact, it does
> present some challenges that are not well solved right now, if you
> unbind the Ethernet MAC driver, you leave a dangling DSA switch tree,
> and good luck re-attaching it.
> 
> So maybe we should talk about it in person at $next conference, I will
> be at ELC, how about you?

No, sorry, I'll be at a conference all the week before, and was at
FOSDEM, conferences 3 weeks in a row is rough.

greg k-h