netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices
@ 2015-07-13  4:48 Punnaiah Choudary Kalluri
  2015-07-13  4:48 ` [RFC PATCH 1/2] " Punnaiah Choudary Kalluri
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Punnaiah Choudary Kalluri @ 2015-07-13  4:48 UTC (permalink / raw)
  To: nicolas.ferre, michals, anirudh, davem
  Cc: harinik, kpc528, kalluripunnaiahchoudary, netdev,
	Punnaiah Choudary Kalluri

This patch is to add support for the design that has multiple ethernet
mac controllers and single mdio bus connected to multiple phy devices.
i.e mdio lines are connected to any of the ethernet mac controller and
all the phy devices will be accessed using the phy maintenance interface
in that mac controller.

 ______                   _____
|      |                 |PHY0 |
| MAC0 |-----------------|     |
|______|       |         |_____|
               |           
 ______        |          _____
|      |       |         |     |
| MAC1 |       |_________|PHY1 | 
|______|                 |____ |

So, i come up with two implementations for addressing the above configuration.

Implementation 1:
 Have separate driver for mdio bus
 Create a DT node for all the PHY devices connected to the mdio bus
 This driver will share the register space of the mac controller that has
 mdio bus connected.  

Implementation 2:
 Add new property "has-mdio" and it should be 1 for the mac that has mdio bus
 connected.
 Create the mdio bus only when the has-mdio property is 1

Please review the two implementations and suggest which one is better to proceed
further. In my opinion implementation 1 will be the ideal one.

Currently i have tested the patches with single mac and single phy
configuration. I need to take care of few more cases before releasing the final patch
but before that i would like to have your opinion on the above implementations
and finalize one implementation. so that i can enhance it further.

Punnaiah Choudary Kalluri (1):
  net: macb: Add mdio driver for accessing multiple phy devices
  net: macb: Add support for single mac managing more than one phy


 drivers/net/ethernet/cadence/Makefile    |    2 +-
 drivers/net/ethernet/cadence/macb.c      |   93 +-------------
 drivers/net/ethernet/cadence/macb.h      |    3 +-
 drivers/net/ethernet/cadence/macb_mdio.c |  204 ++++++++++++++++++++++++++++++
 4 files changed, 211 insertions(+), 91 deletions(-)
 create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c

-- 
1.7.4

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

* [RFC PATCH 1/2] net: macb: Add mdio driver for accessing multiple phy devices
  2015-07-13  4:48 [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices Punnaiah Choudary Kalluri
@ 2015-07-13  4:48 ` Punnaiah Choudary Kalluri
  2015-07-13  4:48 ` [RFC PATCH 2/2] net: macb: Add support for single mac managing more than one phy Punnaiah Choudary Kalluri
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: Punnaiah Choudary Kalluri @ 2015-07-13  4:48 UTC (permalink / raw)
  To: nicolas.ferre, michals, anirudh, davem
  Cc: harinik, kpc528, kalluripunnaiahchoudary, netdev,
	Punnaiah Choudary Kalluri

This patch is to add spoort for the design that has multiple ethernet
mac controllers and single mdio bus connected to multiple phy devices.
i.e mdio lines are connected to any of the ethernet mac controller and
all the phy devices will be accessed using the phy maintainance interface
in that mac controller.

Signed-off-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
---
 drivers/net/ethernet/cadence/Makefile    |    2 +-
 drivers/net/ethernet/cadence/macb.c      |   93 +-------------
 drivers/net/ethernet/cadence/macb.h      |    3 +-
 drivers/net/ethernet/cadence/macb_mdio.c |  204 ++++++++++++++++++++++++++++++
 4 files changed, 211 insertions(+), 91 deletions(-)
 create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c

diff --git a/drivers/net/ethernet/cadence/Makefile b/drivers/net/ethernet/cadence/Makefile
index 9068b83..73504f4 100644
--- a/drivers/net/ethernet/cadence/Makefile
+++ b/drivers/net/ethernet/cadence/Makefile
@@ -3,4 +3,4 @@
 #
 
 obj-$(CONFIG_ARM_AT91_ETHER) += at91_ether.o
-obj-$(CONFIG_MACB) += macb.o
+obj-$(CONFIG_MACB) += macb.o macb_mdio.o
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 4833ba1..df1b928 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -320,7 +320,7 @@ static int macb_mii_probe(struct net_device *dev)
 	int phy_irq;
 	int ret;
 
-	phydev = phy_find_first(bp->mii_bus);
+	phydev = of_phy_find_device(bp->phy_node);
 	if (!phydev) {
 		netdev_err(dev, "no PHY found\n");
 		return -ENXIO;
@@ -359,89 +359,6 @@ static int macb_mii_probe(struct net_device *dev)
 	return 0;
 }
 
-int macb_mii_init(struct macb *bp)
-{
-	struct macb_platform_data *pdata;
-	struct device_node *np;
-	int err = -ENXIO, i;
-
-	/* Enable management port */
-	macb_writel(bp, NCR, MACB_BIT(MPE));
-
-	bp->mii_bus = mdiobus_alloc();
-	if (bp->mii_bus == NULL) {
-		err = -ENOMEM;
-		goto err_out;
-	}
-
-	bp->mii_bus->name = "MACB_mii_bus";
-	bp->mii_bus->read = &macb_mdio_read;
-	bp->mii_bus->write = &macb_mdio_write;
-	snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
-		bp->pdev->name, bp->pdev->id);
-	bp->mii_bus->priv = bp;
-	bp->mii_bus->parent = &bp->dev->dev;
-	pdata = dev_get_platdata(&bp->pdev->dev);
-
-	bp->mii_bus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
-	if (!bp->mii_bus->irq) {
-		err = -ENOMEM;
-		goto err_out_free_mdiobus;
-	}
-
-	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
-
-	np = bp->pdev->dev.of_node;
-	if (np) {
-		/* try dt phy registration */
-		err = of_mdiobus_register(bp->mii_bus, np);
-
-		/* fallback to standard phy registration if no phy were
-		   found during dt phy registration */
-		if (!err && !phy_find_first(bp->mii_bus)) {
-			for (i = 0; i < PHY_MAX_ADDR; i++) {
-				struct phy_device *phydev;
-
-				phydev = mdiobus_scan(bp->mii_bus, i);
-				if (IS_ERR(phydev)) {
-					err = PTR_ERR(phydev);
-					break;
-				}
-			}
-
-			if (err)
-				goto err_out_unregister_bus;
-		}
-	} else {
-		for (i = 0; i < PHY_MAX_ADDR; i++)
-			bp->mii_bus->irq[i] = PHY_POLL;
-
-		if (pdata)
-			bp->mii_bus->phy_mask = pdata->phy_mask;
-
-		err = mdiobus_register(bp->mii_bus);
-	}
-
-	if (err)
-		goto err_out_free_mdio_irq;
-
-	err = macb_mii_probe(bp->dev);
-	if (err)
-		goto err_out_unregister_bus;
-
-	return 0;
-
-err_out_unregister_bus:
-	mdiobus_unregister(bp->mii_bus);
-err_out_free_mdio_irq:
-	kfree(bp->mii_bus->irq);
-err_out_free_mdiobus:
-	mdiobus_free(bp->mii_bus);
-err_out:
-	return err;
-}
-EXPORT_SYMBOL_GPL(macb_mii_init);
-
 static void macb_update_stats(struct macb *bp)
 {
 	u32 __iomem *reg = bp->regs + MACB_PFR;
@@ -2480,7 +2397,10 @@ static int macb_probe(struct platform_device *pdev)
 		goto err_out_free_netdev;
 	}
 
-	err = macb_mii_init(bp);
+	bp->phy_node = of_parse_phandle(bp->pdev->dev.of_node,
+						"phy-handle", 0);
+
+	err = macb_mii_probe(bp->dev);
 	if (err)
 		goto err_out_unregister_netdev;
 
@@ -2524,9 +2444,6 @@ static int macb_remove(struct platform_device *pdev)
 		bp = netdev_priv(dev);
 		if (bp->phy_dev)
 			phy_disconnect(bp->phy_dev);
-		mdiobus_unregister(bp->mii_bus);
-		kfree(bp->mii_bus->irq);
-		mdiobus_free(bp->mii_bus);
 		unregister_netdev(dev);
 		if (!IS_ERR(bp->tx_clk))
 			clk_disable_unprepare(bp->tx_clk);
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index f0aa177..ba515ab 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -825,13 +825,12 @@ struct macb {
 	unsigned int		rx_frm_len_mask;
 	unsigned int		jumbo_max_len;
 	bool			isjumbo;
-
+	struct device_node *phy_node;
 	u64			ethtool_stats[GEM_STATS_LEN];
 };
 
 extern const struct ethtool_ops macb_ethtool_ops;
 
-int macb_mii_init(struct macb *bp);
 int macb_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
 struct net_device_stats *macb_get_stats(struct net_device *dev);
 void macb_set_rx_mode(struct net_device *dev);
diff --git a/drivers/net/ethernet/cadence/macb_mdio.c b/drivers/net/ethernet/cadence/macb_mdio.c
new file mode 100644
index 0000000..563ac52
--- /dev/null
+++ b/drivers/net/ethernet/cadence/macb_mdio.c
@@ -0,0 +1,204 @@
+/*
+ * Cadence Macb mdio controller driver
+ *
+ * Copyright (C) 2014 - 2015 Xilinx, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License version 2 as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/netdevice.h>
+#include <linux/of_address.h>
+#include <linux/of_mdio.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include "macb.h"
+
+struct macb_mdio_data {
+	void __iomem *regs;
+	struct clk *pclk;
+	struct clk *hclk;
+};
+
+static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
+{
+	struct macb_mdio_data *bp = bus->priv;
+	int value;
+
+	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
+			      | MACB_BF(RW, MACB_MAN_READ)
+			      | MACB_BF(PHYA, mii_id)
+			      | MACB_BF(REGA, regnum)
+			      | MACB_BF(CODE, MACB_MAN_CODE)));
+
+	/* wait for end of transfer */
+	while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
+		cpu_relax();
+
+	value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
+
+	return value;
+}
+
+static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
+			   u16 value)
+{
+	struct macb_mdio_data *bp = bus->priv;
+
+	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
+			      | MACB_BF(RW, MACB_MAN_WRITE)
+			      | MACB_BF(PHYA, mii_id)
+			      | MACB_BF(REGA, regnum)
+			      | MACB_BF(CODE, MACB_MAN_CODE)
+			      | MACB_BF(DATA, value)));
+
+	/* wait for end of transfer */
+	while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
+		cpu_relax();
+
+	return 0;
+}
+
+static u32 gem_mdc_clk_div(struct macb_mdio_data *bp)
+{
+	u32 config;
+	unsigned long pclk_hz = clk_get_rate(bp->pclk);
+
+	if (pclk_hz <= 20000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV8);
+	else if (pclk_hz <= 40000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV16);
+	else if (pclk_hz <= 80000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV32);
+	else if (pclk_hz <= 120000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV48);
+	else if (pclk_hz <= 160000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV64);
+	else
+		config = GEM_BF(CLK, GEM_CLK_DIV96);
+
+	return config;
+}
+
+static int macb_mdio_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct mii_bus *bus;
+	struct macb_mdio_data *bp;
+	struct resource *res;
+	int ret;
+	u32 config, i;
+
+	bus = mdiobus_alloc_size(sizeof(*bp));
+	if (!bus)
+		return -ENOMEM;
+
+	bus->name = "macb_mii_bus";
+	bus->read = &macb_mdio_read;
+	bus->write = &macb_mdio_write;
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(&pdev->dev));
+	bus->parent = &pdev->dev;
+	bus->irq = devm_kzalloc(&pdev->dev, sizeof(int) * PHY_MAX_ADDR,
+				GFP_KERNEL);
+	if (!bus->irq) {
+		ret = -ENOMEM;
+		goto err_out_free_mdiobus;
+	}
+
+	for (i = 0; i < PHY_MAX_ADDR; i++)
+		bus->irq[i] = PHY_POLL;
+
+	bp = bus->priv;
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	bp->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(bp->regs)) {
+		ret = PTR_ERR(bp->regs);
+		goto err_out_free_mdiobus;
+	}
+
+	bp->pclk = devm_clk_get(&pdev->dev, "pclk");
+	if (IS_ERR(bp->pclk)) {
+		ret = PTR_ERR(bp->pclk);
+		dev_err(&pdev->dev, "failed to get macb_clk (%u)\n", ret);
+		goto err_out_free_mdiobus;
+	}
+
+	bp->hclk = devm_clk_get(&pdev->dev, "hclk");
+	if (IS_ERR(bp->hclk)) {
+		ret = PTR_ERR(bp->hclk);
+		dev_err(&pdev->dev, "failed to get hclk (%u)\n", ret);
+		goto err_out_free_mdiobus;
+	}
+
+	ret = clk_prepare_enable(bp->pclk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable pclk (%u)\n", ret);
+		goto err_out_free_mdiobus;
+	}
+
+	ret = clk_prepare_enable(bp->hclk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable hclk (%u)\n", ret);
+		goto err_disable_pclk;
+	}
+
+	/* Enable management port */
+	macb_writel(bp, NCR, MACB_BIT(MPE));
+	config = gem_mdc_clk_div(bp);
+	macb_writel(bp, NCFGR, config);
+
+	ret = of_mdiobus_register(bus, np);
+	if (ret < 0)
+		goto err_out_free_mdiobus;
+
+	platform_set_drvdata(pdev, bus);
+
+	return 0;
+
+err_disable_pclk:
+	clk_disable_unprepare(bp->pclk);
+
+err_out_free_mdiobus:
+	mdiobus_free(bus);
+	return ret;
+}
+
+static int macb_mdio_remove(struct platform_device *pdev)
+{
+	struct mii_bus *bus = platform_get_drvdata(pdev);
+	struct macb_mdio_data *bp = bus->priv;
+
+	mdiobus_unregister(bus);
+	clk_disable_unprepare(bp->hclk);
+	clk_disable_unprepare(bp->pclk);
+	mdiobus_free(bus);
+
+	return 0;
+}
+
+static const struct of_device_id macb_mdio_dt_ids[] = {
+	{ .compatible = "cdns,macb-mdio" },
+
+};
+MODULE_DEVICE_TABLE(of, macb_mdio_dt_ids);
+
+static struct platform_driver macb_mdio_driver = {
+	.probe = macb_mdio_probe,
+	.remove = macb_mdio_remove,
+	.driver = {
+		.name = "macb-mdio",
+		.of_match_table = macb_mdio_dt_ids,
+	},
+};
+
+module_platform_driver(macb_mdio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Cadence MACB/GEM Ethernet driver");
+MODULE_AUTHOR("Xilinx");
-- 
1.7.4

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

* [RFC PATCH 2/2] net: macb: Add support for single mac managing more than one phy
  2015-07-13  4:48 [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices Punnaiah Choudary Kalluri
  2015-07-13  4:48 ` [RFC PATCH 1/2] " Punnaiah Choudary Kalluri
@ 2015-07-13  4:48 ` Punnaiah Choudary Kalluri
  2015-07-13 18:43 ` [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices Florian Fainelli
  2015-07-20 13:30 ` Michal Simek
  3 siblings, 0 replies; 16+ messages in thread
From: Punnaiah Choudary Kalluri @ 2015-07-13  4:48 UTC (permalink / raw)
  To: nicolas.ferre, michals, anirudh, davem
  Cc: harinik, kpc528, kalluripunnaiahchoudary, netdev,
	Punnaiah Choudary Kalluri

Added support for single mac managing more than one phy

Signed-off-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
---
 drivers/net/ethernet/cadence/macb.c |   25 ++++++++++++++++++++-----
 drivers/net/ethernet/cadence/macb.h |    4 +++-
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 4833ba1..6d36b76 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -171,6 +171,7 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 	struct macb *bp = bus->priv;
 	int value;
 
+	spin_lock(&bp->mdio_lock);
 	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
 			      | MACB_BF(RW, MACB_MAN_READ)
 			      | MACB_BF(PHYA, mii_id)
@@ -182,6 +183,7 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 		cpu_relax();
 
 	value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
+	spin_unlock(&bp->mdio_lock);
 
 	return value;
 }
@@ -191,6 +193,7 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
 {
 	struct macb *bp = bus->priv;
 
+	spin_lock(&bp->mdio_lock);
 	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
 			      | MACB_BF(RW, MACB_MAN_WRITE)
 			      | MACB_BF(PHYA, mii_id)
@@ -201,6 +204,7 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
 	/* wait for end of transfer */
 	while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
 		cpu_relax();
+	spin_unlock(&bp->mdio_lock);
 
 	return 0;
 }
@@ -320,7 +324,7 @@ static int macb_mii_probe(struct net_device *dev)
 	int phy_irq;
 	int ret;
 
-	phydev = phy_find_first(bp->mii_bus);
+	phydev = of_phy_find_device(bp->phy_node);
 	if (!phydev) {
 		netdev_err(dev, "no PHY found\n");
 		return -ENXIO;
@@ -365,8 +369,14 @@ int macb_mii_init(struct macb *bp)
 	struct device_node *np;
 	int err = -ENXIO, i;
 
+	bp->phy_node = of_parse_phandle(bp->pdev->dev.of_node,
+						"phy-handle", 0);
+	np = of_get_parent(bp->phy_node);
 	/* Enable management port */
 	macb_writel(bp, NCR, MACB_BIT(MPE));
+	bp->mii_bus = of_mdio_find_bus(np);
+	if (!bp->has_mdio && bp->mii_bus)
+		goto mii_probe;
 
 	bp->mii_bus = mdiobus_alloc();
 	if (bp->mii_bus == NULL) {
@@ -425,6 +435,7 @@ int macb_mii_init(struct macb *bp)
 	if (err)
 		goto err_out_free_mdio_irq;
 
+mii_probe:
 	err = macb_mii_probe(bp->dev);
 	if (err)
 		goto err_out_unregister_bus;
@@ -2356,6 +2367,7 @@ static int macb_probe(struct platform_device *pdev)
 	bp->isjumbo = of_property_read_bool(pdev->dev.of_node,
 					    "jumbo-supported");
 	spin_lock_init(&bp->lock);
+	spin_lock_init(&bp->mdio_lock);
 
 	/* set the queue register mapping once for all: queue0 has a special
 	 * register mapping but we don't want to test the queue index then
@@ -2479,7 +2491,8 @@ static int macb_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "Cannot register net device, aborting.\n");
 		goto err_out_free_netdev;
 	}
-
+	err = of_property_read_u32(bp->pdev->dev.of_node, "has-mdio",
+							&bp->has_mdio);
 	err = macb_mii_init(bp);
 	if (err)
 		goto err_out_unregister_netdev;
@@ -2524,9 +2537,11 @@ static int macb_remove(struct platform_device *pdev)
 		bp = netdev_priv(dev);
 		if (bp->phy_dev)
 			phy_disconnect(bp->phy_dev);
-		mdiobus_unregister(bp->mii_bus);
-		kfree(bp->mii_bus->irq);
-		mdiobus_free(bp->mii_bus);
+		if (bp->has_mdio) {
+			mdiobus_unregister(bp->mii_bus);
+			kfree(bp->mii_bus->irq);
+			mdiobus_free(bp->mii_bus);
+		}
 		unregister_netdev(dev);
 		if (!IS_ERR(bp->tx_clk))
 			clk_disable_unprepare(bp->tx_clk);
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index f0aa177..0f99f2a 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -825,7 +825,9 @@ struct macb {
 	unsigned int		rx_frm_len_mask;
 	unsigned int		jumbo_max_len;
 	bool			isjumbo;
-
+	unsigned int has_mdio;
+	spinlock_t mdio_lock;
+	struct device_node *phy_node;
 	u64			ethtool_stats[GEM_STATS_LEN];
 };
 
-- 
1.7.4

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

* Re: [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices
  2015-07-13  4:48 [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices Punnaiah Choudary Kalluri
  2015-07-13  4:48 ` [RFC PATCH 1/2] " Punnaiah Choudary Kalluri
  2015-07-13  4:48 ` [RFC PATCH 2/2] net: macb: Add support for single mac managing more than one phy Punnaiah Choudary Kalluri
@ 2015-07-13 18:43 ` Florian Fainelli
  2015-07-14  3:02   ` punnaiah choudary kalluri
  2015-07-20 13:30 ` Michal Simek
  3 siblings, 1 reply; 16+ messages in thread
From: Florian Fainelli @ 2015-07-13 18:43 UTC (permalink / raw)
  To: Punnaiah Choudary Kalluri, nicolas.ferre, michals, anirudh, davem
  Cc: harinik, kpc528, kalluripunnaiahchoudary, netdev,
	Punnaiah Choudary Kalluri

On 12/07/15 21:48, Punnaiah Choudary Kalluri wrote:
> This patch is to add support for the design that has multiple ethernet
> mac controllers and single mdio bus connected to multiple phy devices.
> i.e mdio lines are connected to any of the ethernet mac controller and
> all the phy devices will be accessed using the phy maintenance interface
> in that mac controller.
> 
>  ______                   _____
> |      |                 |PHY0 |
> | MAC0 |-----------------|     |
> |______|       |         |_____|
>                |           
>  ______        |          _____
> |      |       |         |     |
> | MAC1 |       |_________|PHY1 | 
> |______|                 |____ |
> 
> So, i come up with two implementations for addressing the above configuration.
> 
> Implementation 1:
>  Have separate driver for mdio bus
>  Create a DT node for all the PHY devices connected to the mdio bus
>  This driver will share the register space of the mac controller that has
>  mdio bus connected.

That is the best design implementation, MDIO in itself is a sub-piece of
your Ethernet MAC controller the fact that it is within the Ethernet MAC
core is just coincidental, but there is no reason why it could not be
taken apart and made a separate block in itself.

> 
> Implementation 2:
>  Add new property "has-mdio" and it should be 1 for the mac that has mdio bus
>  connected.
>  Create the mdio bus only when the has-mdio property is 1
> 
> Please review the two implementations and suggest which one is better to proceed
> further. In my opinion implementation 1 will be the ideal one.

Agreed.

> 
> Currently i have tested the patches with single mac and single phy
> configuration. I need to take care of few more cases before releasing the final patch
> but before that i would like to have your opinion on the above implementations
> and finalize one implementation. so that i can enhance it further.
> 
> Punnaiah Choudary Kalluri (1):
>   net: macb: Add mdio driver for accessing multiple phy devices
>   net: macb: Add support for single mac managing more than one phy
> 
> 
>  drivers/net/ethernet/cadence/Makefile    |    2 +-
>  drivers/net/ethernet/cadence/macb.c      |   93 +-------------
>  drivers/net/ethernet/cadence/macb.h      |    3 +-
>  drivers/net/ethernet/cadence/macb_mdio.c |  204 ++++++++++++++++++++++++++++++
>  4 files changed, 211 insertions(+), 91 deletions(-)
>  create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
> 


-- 
Florian

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

* Re: [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices
  2015-07-13 18:43 ` [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices Florian Fainelli
@ 2015-07-14  3:02   ` punnaiah choudary kalluri
  0 siblings, 0 replies; 16+ messages in thread
From: punnaiah choudary kalluri @ 2015-07-14  3:02 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Punnaiah Choudary Kalluri, nicolas.ferre, michals, anirudh,
	davem, harinik, Punnaiah Choudary, netdev

On Tue, Jul 14, 2015 at 12:13 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 12/07/15 21:48, Punnaiah Choudary Kalluri wrote:
>> This patch is to add support for the design that has multiple ethernet
>> mac controllers and single mdio bus connected to multiple phy devices.
>> i.e mdio lines are connected to any of the ethernet mac controller and
>> all the phy devices will be accessed using the phy maintenance interface
>> in that mac controller.
>>
>>  ______                   _____
>> |      |                 |PHY0 |
>> | MAC0 |-----------------|     |
>> |______|       |         |_____|
>>                |
>>  ______        |          _____
>> |      |       |         |     |
>> | MAC1 |       |_________|PHY1 |
>> |______|                 |____ |
>>
>> So, i come up with two implementations for addressing the above configuration.
>>
>> Implementation 1:
>>  Have separate driver for mdio bus
>>  Create a DT node for all the PHY devices connected to the mdio bus
>>  This driver will share the register space of the mac controller that has
>>  mdio bus connected.
>
> That is the best design implementation, MDIO in itself is a sub-piece of
> your Ethernet MAC controller the fact that it is within the Ethernet MAC
> core is just coincidental, but there is no reason why it could not be
> taken apart and made a separate block in itself.

Thanks Florian for suggesting this.
No idea on why the mdio block was not made a separate block.

regards,
Punnaiah

>
>>
>> Implementation 2:
>>  Add new property "has-mdio" and it should be 1 for the mac that has mdio bus
>>  connected.
>>  Create the mdio bus only when the has-mdio property is 1
>>
>> Please review the two implementations and suggest which one is better to proceed
>> further. In my opinion implementation 1 will be the ideal one.
>
> Agreed.
>
>>
>> Currently i have tested the patches with single mac and single phy
>> configuration. I need to take care of few more cases before releasing the final patch
>> but before that i would like to have your opinion on the above implementations
>> and finalize one implementation. so that i can enhance it further.
>>
>> Punnaiah Choudary Kalluri (1):
>>   net: macb: Add mdio driver for accessing multiple phy devices
>>   net: macb: Add support for single mac managing more than one phy
>>
>>
>>  drivers/net/ethernet/cadence/Makefile    |    2 +-
>>  drivers/net/ethernet/cadence/macb.c      |   93 +-------------
>>  drivers/net/ethernet/cadence/macb.h      |    3 +-
>>  drivers/net/ethernet/cadence/macb_mdio.c |  204 ++++++++++++++++++++++++++++++
>>  4 files changed, 211 insertions(+), 91 deletions(-)
>>  create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
>>
>
>
> --
> Florian

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

* Re: [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices
  2015-07-13  4:48 [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices Punnaiah Choudary Kalluri
                   ` (2 preceding siblings ...)
  2015-07-13 18:43 ` [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices Florian Fainelli
@ 2015-07-20 13:30 ` Michal Simek
  2015-07-20 16:23   ` Andrew Lunn
  2015-07-27  7:37   ` Nicolas Ferre
  3 siblings, 2 replies; 16+ messages in thread
From: Michal Simek @ 2015-07-20 13:30 UTC (permalink / raw)
  To: Punnaiah Choudary Kalluri, nicolas.ferre, anirudh, davem
  Cc: harinik, kpc528, kalluripunnaiahchoudary, netdev,
	Punnaiah Choudary Kalluri

Hi Nicolas,

have you had a time to look at this?

Thanks,
Michal

On 07/13/2015 06:48 AM, Punnaiah Choudary Kalluri wrote:
> This patch is to add support for the design that has multiple ethernet
> mac controllers and single mdio bus connected to multiple phy devices.
> i.e mdio lines are connected to any of the ethernet mac controller and
> all the phy devices will be accessed using the phy maintenance interface
> in that mac controller.
> 
>  ______                   _____
> |      |                 |PHY0 |
> | MAC0 |-----------------|     |
> |______|       |         |_____|
>                |           
>  ______        |          _____
> |      |       |         |     |
> | MAC1 |       |_________|PHY1 | 
> |______|                 |____ |
> 
> So, i come up with two implementations for addressing the above configuration.
> 
> Implementation 1:
>  Have separate driver for mdio bus
>  Create a DT node for all the PHY devices connected to the mdio bus
>  This driver will share the register space of the mac controller that has
>  mdio bus connected.  
> 
> Implementation 2:
>  Add new property "has-mdio" and it should be 1 for the mac that has mdio bus
>  connected.
>  Create the mdio bus only when the has-mdio property is 1
> 
> Please review the two implementations and suggest which one is better to proceed
> further. In my opinion implementation 1 will be the ideal one.
> 
> Currently i have tested the patches with single mac and single phy
> configuration. I need to take care of few more cases before releasing the final patch
> but before that i would like to have your opinion on the above implementations
> and finalize one implementation. so that i can enhance it further.
> 
> Punnaiah Choudary Kalluri (1):
>   net: macb: Add mdio driver for accessing multiple phy devices
>   net: macb: Add support for single mac managing more than one phy
> 
> 
>  drivers/net/ethernet/cadence/Makefile    |    2 +-
>  drivers/net/ethernet/cadence/macb.c      |   93 +-------------
>  drivers/net/ethernet/cadence/macb.h      |    3 +-
>  drivers/net/ethernet/cadence/macb_mdio.c |  204 ++++++++++++++++++++++++++++++
>  4 files changed, 211 insertions(+), 91 deletions(-)
>  create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
> 

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

* Re: [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices
  2015-07-20 13:30 ` Michal Simek
@ 2015-07-20 16:23   ` Andrew Lunn
  2015-07-27  7:37   ` Nicolas Ferre
  1 sibling, 0 replies; 16+ messages in thread
From: Andrew Lunn @ 2015-07-20 16:23 UTC (permalink / raw)
  To: Michal Simek
  Cc: Punnaiah Choudary Kalluri, nicolas.ferre, anirudh, davem,
	harinik, kpc528, kalluripunnaiahchoudary, netdev,
	Punnaiah Choudary Kalluri

On Mon, Jul 20, 2015 at 03:30:36PM +0200, Michal Simek wrote:
> Hi Nicolas,
> 
> have you had a time to look at this?
> 
> Thanks,
> Michal
> 
> On 07/13/2015 06:48 AM, Punnaiah Choudary Kalluri wrote:
> > This patch is to add support for the design that has multiple ethernet
> > mac controllers and single mdio bus connected to multiple phy devices.
> > i.e mdio lines are connected to any of the ethernet mac controller and
> > all the phy devices will be accessed using the phy maintenance interface
> > in that mac controller.
> > 
> >  ______                   _____
> > |      |                 |PHY0 |
> > | MAC0 |-----------------|     |
> > |______|       |         |_____|
> >                |           
> >  ______        |          _____
> > |      |       |         |     |
> > | MAC1 |       |_________|PHY1 | 
> > |______|                 |____ |
> > 
> > So, i come up with two implementations for addressing the above configuration.
> > 
> > Implementation 1:
> >  Have separate driver for mdio bus
> >  Create a DT node for all the PHY devices connected to the mdio bus
> >  This driver will share the register space of the mac controller that has
> >  mdio bus connected.  
> > 

Hi Michal

The above it what Marvell, Freescale FEC and probably others do. It is
well defined in Documentation/devicetree/bindings/net/ethernet.txt
that you can have a phy-handle property containing a phandle to the
actual phy device on some random MDIO bus.

     Andrew

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

* Re: [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices
  2015-07-20 13:30 ` Michal Simek
  2015-07-20 16:23   ` Andrew Lunn
@ 2015-07-27  7:37   ` Nicolas Ferre
  2015-07-28  3:34     ` Punnaiah Choudary Kalluri
  1 sibling, 1 reply; 16+ messages in thread
From: Nicolas Ferre @ 2015-07-27  7:37 UTC (permalink / raw)
  To: Michal Simek, Punnaiah Choudary Kalluri, anirudh, davem,
	Florian Fainelli, andrew
  Cc: harinik, kpc528, kalluripunnaiahchoudary, netdev,
	Punnaiah Choudary Kalluri

Le 20/07/2015 15:30, Michal Simek a écrit :
> Hi Nicolas,
> 
> have you had a time to look at this?

Michal,

Sorry for the delay.

With the insight of Florian and Andrew, I do agree to move to your
"Implementation 1" and a separate driver for mdio bus.

The only thing that worries me it that the architecture AVR32 which we
are sharing this driver with doesn't have support for device tree (and
may never have). So, it would be good to make sure to keep compatibility
with it.

Another point to take into account is the transition period: we have
several SoC .dtsi/dts that will need to be converted and we'll need to
add this mdio support before that it's integrated in netdev: we have to
think about it in advance.

Thanks, bye.


> On 07/13/2015 06:48 AM, Punnaiah Choudary Kalluri wrote:
>> This patch is to add support for the design that has multiple ethernet
>> mac controllers and single mdio bus connected to multiple phy devices.
>> i.e mdio lines are connected to any of the ethernet mac controller and
>> all the phy devices will be accessed using the phy maintenance interface
>> in that mac controller.
>>
>>  ______                   _____
>> |      |                 |PHY0 |
>> | MAC0 |-----------------|     |
>> |______|       |         |_____|
>>                |           
>>  ______        |          _____
>> |      |       |         |     |
>> | MAC1 |       |_________|PHY1 | 
>> |______|                 |____ |
>>
>> So, i come up with two implementations for addressing the above configuration.
>>
>> Implementation 1:
>>  Have separate driver for mdio bus
>>  Create a DT node for all the PHY devices connected to the mdio bus
>>  This driver will share the register space of the mac controller that has
>>  mdio bus connected.  
>>
>> Implementation 2:
>>  Add new property "has-mdio" and it should be 1 for the mac that has mdio bus
>>  connected.
>>  Create the mdio bus only when the has-mdio property is 1
>>
>> Please review the two implementations and suggest which one is better to proceed
>> further. In my opinion implementation 1 will be the ideal one.
>>
>> Currently i have tested the patches with single mac and single phy
>> configuration. I need to take care of few more cases before releasing the final patch
>> but before that i would like to have your opinion on the above implementations
>> and finalize one implementation. so that i can enhance it further.
>>
>> Punnaiah Choudary Kalluri (1):
>>   net: macb: Add mdio driver for accessing multiple phy devices
>>   net: macb: Add support for single mac managing more than one phy
>>
>>
>>  drivers/net/ethernet/cadence/Makefile    |    2 +-
>>  drivers/net/ethernet/cadence/macb.c      |   93 +-------------
>>  drivers/net/ethernet/cadence/macb.h      |    3 +-
>>  drivers/net/ethernet/cadence/macb_mdio.c |  204 ++++++++++++++++++++++++++++++
>>  4 files changed, 211 insertions(+), 91 deletions(-)
>>  create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
>>
> 
> 
> 


-- 
Nicolas Ferre

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

* RE: [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices
  2015-07-27  7:37   ` Nicolas Ferre
@ 2015-07-28  3:34     ` Punnaiah Choudary Kalluri
  2015-07-31 21:53       ` Nathan Sullivan
  2015-07-31 21:58       ` Nathan Sullivan
  0 siblings, 2 replies; 16+ messages in thread
From: Punnaiah Choudary Kalluri @ 2015-07-28  3:34 UTC (permalink / raw)
  To: Nicolas Ferre, Michal Simek, Anirudha Sarangi, davem,
	Florian Fainelli, andrew
  Cc: Harini Katakam, kpc528, kalluripunnaiahchoudary, netdev

Hi Nicolas,

> -----Original Message-----
> From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]
> Sent: Monday, July 27, 2015 1:07 PM
> To: Michal Simek; Punnaiah Choudary Kalluri; Anirudha Sarangi;
> davem@davemloft.net; Florian Fainelli; andrew@lunn.ch
> Cc: Harini Katakam; kpc528@gmail.com;
> kalluripunnaiahchoudary@gmail.com; netdev@vger.kernel.org; Punnaiah
> Choudary Kalluri
> Subject: Re: [RFC PATCH 0/2] net: macb: Add mdio driver for accessing
> multiple phy devices
> 
> Le 20/07/2015 15:30, Michal Simek a écrit :
> > Hi Nicolas,
> >
> > have you had a time to look at this?
> 
> Michal,
> 
> Sorry for the delay.
> 
> With the insight of Florian and Andrew, I do agree to move to your
> "Implementation 1" and a separate driver for mdio bus.
> 
> The only thing that worries me it that the architecture AVR32 which we
> are sharing this driver with doesn't have support for device tree (and
> may never have). So, it would be good to make sure to keep compatibility
> with it.
> 
> Another point to take into account is the transition period: we have
> several SoC .dtsi/dts that will need to be converted and we'll need to
> add this mdio support before that it's integrated in netdev: we have to
> think about it in advance.

Ok. I will send you updated patch for mdio support soon and we will finalize next
Course of actions if it doesn't break the existing flow. 

Thanks,
Punnaiah
> 
> Thanks, bye.
> 
> 
> > On 07/13/2015 06:48 AM, Punnaiah Choudary Kalluri wrote:
> >> This patch is to add support for the design that has multiple ethernet
> >> mac controllers and single mdio bus connected to multiple phy devices.
> >> i.e mdio lines are connected to any of the ethernet mac controller and
> >> all the phy devices will be accessed using the phy maintenance interface
> >> in that mac controller.
> >>
> >>  ______                   _____
> >> |      |                 |PHY0 |
> >> | MAC0 |-----------------|     |
> >> |______|       |         |_____|
> >>                |
> >>  ______        |          _____
> >> |      |       |         |     |
> >> | MAC1 |       |_________|PHY1 |
> >> |______|                 |____ |
> >>
> >> So, i come up with two implementations for addressing the above
> configuration.
> >>
> >> Implementation 1:
> >>  Have separate driver for mdio bus
> >>  Create a DT node for all the PHY devices connected to the mdio bus
> >>  This driver will share the register space of the mac controller that has
> >>  mdio bus connected.
> >>
> >> Implementation 2:
> >>  Add new property "has-mdio" and it should be 1 for the mac that has
> mdio bus
> >>  connected.
> >>  Create the mdio bus only when the has-mdio property is 1
> >>
> >> Please review the two implementations and suggest which one is better
> to proceed
> >> further. In my opinion implementation 1 will be the ideal one.
> >>
> >> Currently i have tested the patches with single mac and single phy
> >> configuration. I need to take care of few more cases before releasing the
> final patch
> >> but before that i would like to have your opinion on the above
> implementations
> >> and finalize one implementation. so that i can enhance it further.
> >>
> >> Punnaiah Choudary Kalluri (1):
> >>   net: macb: Add mdio driver for accessing multiple phy devices
> >>   net: macb: Add support for single mac managing more than one phy
> >>
> >>
> >>  drivers/net/ethernet/cadence/Makefile    |    2 +-
> >>  drivers/net/ethernet/cadence/macb.c      |   93 +-------------
> >>  drivers/net/ethernet/cadence/macb.h      |    3 +-
> >>  drivers/net/ethernet/cadence/macb_mdio.c |  204
> ++++++++++++++++++++++++++++++
> >>  4 files changed, 211 insertions(+), 91 deletions(-)
> >>  create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
> >>
> >
> >
> >
> 
> 
> --
> Nicolas Ferre

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

* Re: [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices
  2015-07-28  3:34     ` Punnaiah Choudary Kalluri
@ 2015-07-31 21:53       ` Nathan Sullivan
  2015-08-03  6:01         ` Michal Simek
  2015-07-31 21:58       ` Nathan Sullivan
  1 sibling, 1 reply; 16+ messages in thread
From: Nathan Sullivan @ 2015-07-31 21:53 UTC (permalink / raw)
  To: Punnaiah Choudary Kalluri
  Cc: Nicolas Ferre, Michal Simek, Anirudha Sarangi, davem,
	Florian Fainelli, andrew, Harini Katakam, kpc528,
	kalluripunnaiahchoudary, netdev

On Tue, Jul 28, 2015 at 03:34:51AM +0000, Punnaiah Choudary Kalluri wrote:
> Ok. I will send you updated patch for mdio support soon and we will finalize next
> Course of actions if it doesn't break the existing flow. 
> 
> Thanks,
> Punnaiah

Just a heads up, when mdio no longer turns off when macb goes down, the micrel
9031 phy will have an issue with interrupts getting disabling during phy
suspend.  I have a patch to correct this issue here:

https://patchwork.ozlabs.org/patch/502189/

Would you mind including this patch in your set?

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

* Re: [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices
  2015-07-28  3:34     ` Punnaiah Choudary Kalluri
  2015-07-31 21:53       ` Nathan Sullivan
@ 2015-07-31 21:58       ` Nathan Sullivan
  1 sibling, 0 replies; 16+ messages in thread
From: Nathan Sullivan @ 2015-07-31 21:58 UTC (permalink / raw)
  To: Punnaiah Choudary Kalluri
  Cc: Nicolas Ferre, Michal Simek, Anirudha Sarangi, davem,
	Florian Fainelli, andrew, Harini Katakam, kpc528,
	kalluripunnaiahchoudary, netdev

On Tue, Jul 28, 2015 at 03:34:51AM +0000, Punnaiah Choudary Kalluri wrote:
> 
> Ok. I will send you updated patch for mdio support soon and we will finalize next
> Course of actions if it doesn't break the existing flow. 
> 
> Thanks,
> Punnaiah

When you submit this patch and mdio is seperate from the cadence macb driver,
it will likely cause problems with the ksz9031 phy due to it turning interrupts
off when suspended.  Currently, this is not a problem because taking macb down
turns the management port off.  I have a patch to correct this here:

https://patchwork.ozlabs.org/patch/502189/

Would you mind including this patch in your set?

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

* Re: [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices
  2015-07-31 21:53       ` Nathan Sullivan
@ 2015-08-03  6:01         ` Michal Simek
  0 siblings, 0 replies; 16+ messages in thread
From: Michal Simek @ 2015-08-03  6:01 UTC (permalink / raw)
  To: Nathan Sullivan, Punnaiah Choudary Kalluri
  Cc: Nicolas Ferre, Anirudha Sarangi, davem, Florian Fainelli, andrew,
	Harini Katakam, kpc528, kalluripunnaiahchoudary, netdev

Hi,

On 07/31/2015 11:53 PM, Nathan Sullivan wrote:
> On Tue, Jul 28, 2015 at 03:34:51AM +0000, Punnaiah Choudary Kalluri wrote:
>> Ok. I will send you updated patch for mdio support soon and we will finalize next
>> Course of actions if it doesn't break the existing flow. 
>>
>> Thanks,
>> Punnaiah
> 
> Just a heads up, when mdio no longer turns off when macb goes down, the micrel
> 9031 phy will have an issue with interrupts getting disabling during phy
> suspend.  I have a patch to correct this issue here:
> 
> https://patchwork.ozlabs.org/patch/502189/
> 
> Would you mind including this patch in your set?

You should resend the patch again and you got one more argument why this
patch should go it. But it should go in own direction out of this patch.

Thanks,
Michal

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

* Re: [RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
  2016-11-28 16:33   ` Andrew Lunn
@ 2016-11-29  4:11     ` Harini Katakam
  0 siblings, 0 replies; 16+ messages in thread
From: Harini Katakam @ 2016-11-29  4:11 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Harini Katakam, Nicolas Ferre, davem, Rob Herring, Pawel Moll,
	Mark Rutland, ijc+devicetree, Kumar Gala, Boris Brezillon,
	alexandre.belloni, netdev, linux-kernel, devicetree, michals,
	Punnaiah Choudary Kalluri

Hi Andrew,

On Mon, Nov 28, 2016 at 10:03 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Mon, Nov 28, 2016 at 03:19:14PM +0530, Harini Katakam wrote:
>> This patch is to add support for the hardware with multiple ethernet
>> MAC controllers and a single MDIO bus connected to multiple PHY devices.
>> MDIO lines are connected to any one of the ethernet MAC controllers and
>> all the PHY devices will be accessed using the PHY maintenance interface
>> in that MAC controller. This handling along with PHY functionality is
>> moved to macb_mdio.c
>>
>> Signed-off-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
>> Signed-off-by: Harini Katakam <harinik@xilinx.com>
>> ---
>>  drivers/net/ethernet/cadence/Makefile    |   2 +-
>>  drivers/net/ethernet/cadence/macb.c      | 169 +++-----------------
>>  drivers/net/ethernet/cadence/macb.h      |   2 +
>>  drivers/net/ethernet/cadence/macb_mdio.c | 266 +++++++++++++++++++++++++++++++
>>  4 files changed, 294 insertions(+), 145 deletions(-)
>>  create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
>>
<snip>
>> +     bus->irq = devm_kzalloc(&pdev->dev, sizeof(int) * PHY_MAX_ADDR,
>> +                             GFP_KERNEL);
>
> This looks wrong, or at least old. It used to be a pointer to an array,
> but it is now an actual array.

Sorry, this was a mistake.
I changed this after rebase, will update in next version.

>
>> +static const struct of_device_id macb_mdio_dt_ids[] = {
>> +     { .compatible = "cdns,macb-mdio" },
>> +
>> +};
>
>
> I've not looked hard enough to know, but can you keep backwards
> compatibility? Won't old device tree's assume the mdio bus is always
> present? Now you need an explicit node otherwise there will not be an
> mdio bus?

Yes, an explicit MDIO bus is required. But I'm not sure
how to maintain backward compatibility (without using this separate
macb_mdio) and have different MACs use the same MDIO bus
with separate PHYs.

Regards,
Harini

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

* Re: [RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
       [not found] ` <1480326554-6041-1-git-send-email-harinik-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
@ 2016-11-28 16:33   ` Andrew Lunn
  2016-11-29  4:11     ` Harini Katakam
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Lunn @ 2016-11-28 16:33 UTC (permalink / raw)
  To: Harini Katakam
  Cc: nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ,
	boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	harinikatakamlinux-Re5JQEeQqe8AvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	harinik-gjFFaj9aHVfQT0dZR+AlfA, michals-gjFFaj9aHVfQT0dZR+AlfA,
	Punnaiah Choudary Kalluri

On Mon, Nov 28, 2016 at 03:19:14PM +0530, Harini Katakam wrote:
> This patch is to add support for the hardware with multiple ethernet
> MAC controllers and a single MDIO bus connected to multiple PHY devices.
> MDIO lines are connected to any one of the ethernet MAC controllers and
> all the PHY devices will be accessed using the PHY maintenance interface
> in that MAC controller. This handling along with PHY functionality is
> moved to macb_mdio.c
> 
> Signed-off-by: Punnaiah Choudary Kalluri <punnaia-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Harini Katakam <harinik-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/net/ethernet/cadence/Makefile    |   2 +-
>  drivers/net/ethernet/cadence/macb.c      | 169 +++-----------------
>  drivers/net/ethernet/cadence/macb.h      |   2 +
>  drivers/net/ethernet/cadence/macb_mdio.c | 266 +++++++++++++++++++++++++++++++
>  4 files changed, 294 insertions(+), 145 deletions(-)
>  create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
> 
> diff --git a/drivers/net/ethernet/cadence/Makefile b/drivers/net/ethernet/cadence/Makefile
> index 91f79b1..75c3d84 100644
> --- a/drivers/net/ethernet/cadence/Makefile
> +++ b/drivers/net/ethernet/cadence/Makefile
> @@ -2,4 +2,4 @@
>  # Makefile for the Atmel network device drivers.
>  #
>  
> -obj-$(CONFIG_MACB) += macb.o
> +obj-$(CONFIG_MACB) += macb.o macb_mdio.o
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 80ccfc4..ae2a797 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -232,45 +232,6 @@ static void macb_get_hwaddr(struct macb *bp)
>  	eth_hw_addr_random(bp->dev);
>  }
>  
> -static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> -{
> -	struct macb *bp = bus->priv;
> -	int value;
> -
> -	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
> -			      | MACB_BF(RW, MACB_MAN_READ)
> -			      | MACB_BF(PHYA, mii_id)
> -			      | MACB_BF(REGA, regnum)
> -			      | MACB_BF(CODE, MACB_MAN_CODE)));
> -
> -	/* wait for end of transfer */
> -	while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
> -		cpu_relax();
> -
> -	value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
> -
> -	return value;
> -}
> -
> -static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
> -			   u16 value)
> -{
> -	struct macb *bp = bus->priv;
> -
> -	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
> -			      | MACB_BF(RW, MACB_MAN_WRITE)
> -			      | MACB_BF(PHYA, mii_id)
> -			      | MACB_BF(REGA, regnum)
> -			      | MACB_BF(CODE, MACB_MAN_CODE)
> -			      | MACB_BF(DATA, value)));
> -
> -	/* wait for end of transfer */
> -	while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
> -		cpu_relax();
> -
> -	return 0;
> -}
> -
>  /**
>   * macb_set_tx_clk() - Set a clock to a new frequency
>   * @clk		Pointer to the clock to change
> @@ -385,27 +346,19 @@ static void macb_handle_link_change(struct net_device *dev)
>  static int macb_mii_probe(struct net_device *dev)
>  {
>  	struct macb *bp = netdev_priv(dev);
> -	struct macb_platform_data *pdata;
>  	struct phy_device *phydev;
> -	int phy_irq;
>  	int ret;
>  
> -	phydev = phy_find_first(bp->mii_bus);
> +	if (dev->phydev)
> +		return 0;
> +
> +	phydev = of_phy_find_device(bp->phy_node);
>  	if (!phydev) {
>  		netdev_err(dev, "no PHY found\n");
>  		return -ENXIO;
>  	}
> -
> -	pdata = dev_get_platdata(&bp->pdev->dev);
> -	if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
> -		ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin,
> -					"phy int");
> -		if (!ret) {
> -			phy_irq = gpio_to_irq(pdata->phy_irq_pin);
> -			phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
> -		}
> -	}
> -
> +	if (bp->phy_irq)
> +		phydev->irq = bp->phy_irq;
>  	/* attach the mac to the phy */
>  	ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
>  				 bp->phy_interface);
> @@ -429,80 +382,9 @@ static int macb_mii_probe(struct net_device *dev)
>  	bp->speed = 0;
>  	bp->duplex = -1;
>  
> -	return 0;
> -}
> -
> -static int macb_mii_init(struct macb *bp)
> -{
> -	struct macb_platform_data *pdata;
> -	struct device_node *np;
> -	int err = -ENXIO, i;
> -
> -	/* Enable management port */
> -	macb_writel(bp, NCR, MACB_BIT(MPE));
> -
> -	bp->mii_bus = mdiobus_alloc();
> -	if (!bp->mii_bus) {
> -		err = -ENOMEM;
> -		goto err_out;
> -	}
> -
> -	bp->mii_bus->name = "MACB_mii_bus";
> -	bp->mii_bus->read = &macb_mdio_read;
> -	bp->mii_bus->write = &macb_mdio_write;
> -	snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
> -		 bp->pdev->name, bp->pdev->id);
> -	bp->mii_bus->priv = bp;
> -	bp->mii_bus->parent = &bp->pdev->dev;
> -	pdata = dev_get_platdata(&bp->pdev->dev);
> -
> -	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
> -
> -	np = bp->pdev->dev.of_node;
> -	if (np) {
> -		/* try dt phy registration */
> -		err = of_mdiobus_register(bp->mii_bus, np);
> -
> -		/* fallback to standard phy registration if no phy were
> -		 * found during dt phy registration
> -		 */
> -		if (!err && !phy_find_first(bp->mii_bus)) {
> -			for (i = 0; i < PHY_MAX_ADDR; i++) {
> -				struct phy_device *phydev;
> -
> -				phydev = mdiobus_scan(bp->mii_bus, i);
> -				if (IS_ERR(phydev) &&
> -				    PTR_ERR(phydev) != -ENODEV) {
> -					err = PTR_ERR(phydev);
> -					break;
> -				}
> -			}
> -
> -			if (err)
> -				goto err_out_unregister_bus;
> -		}
> -	} else {
> -		if (pdata)
> -			bp->mii_bus->phy_mask = pdata->phy_mask;
> -
> -		err = mdiobus_register(bp->mii_bus);
> -	}
> -
> -	if (err)
> -		goto err_out_free_mdiobus;
> -
> -	err = macb_mii_probe(bp->dev);
> -	if (err)
> -		goto err_out_unregister_bus;
> +	phy_attached_info(phydev);
>  
>  	return 0;
> -
> -err_out_unregister_bus:
> -	mdiobus_unregister(bp->mii_bus);
> -err_out_free_mdiobus:
> -	mdiobus_free(bp->mii_bus);
> -err_out:
> -	return err;
>  }
>  
>  static void macb_update_stats(struct macb *bp)
> @@ -2060,7 +1942,8 @@ static int macb_open(struct net_device *dev)
>  	netif_carrier_off(dev);
>  
>  	/* if the phy is not yet register, retry later*/
> -	if (!dev->phydev)
> +	err = macb_mii_probe(dev);
> +	if (err)
>  		return -EAGAIN;
>  
>  	/* RX buffers initialization */
> @@ -3122,16 +3005,16 @@ static int macb_probe(struct platform_device *pdev)
>  	unsigned int queue_mask, num_queues;
>  	struct macb_platform_data *pdata;
>  	bool native_io;
> -	struct phy_device *phydev;
>  	struct net_device *dev;
>  	struct resource *regs;
>  	void __iomem *mem;
>  	const char *mac;
>  	struct macb *bp;
> +	int phy_irq;
>  	int err;
>  
>  	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	mem = devm_ioremap_resource(&pdev->dev, regs);
> +	mem = devm_ioremap(&pdev->dev, regs->start, resource_size(regs));
>  	if (IS_ERR(mem))
>  		return PTR_ERR(mem);
>  
> @@ -3250,21 +3133,26 @@ static int macb_probe(struct platform_device *pdev)
>  	if (err)
>  		goto err_out_free_netdev;
>  
> -	err = macb_mii_init(bp);
> -	if (err)
> -		goto err_out_free_netdev;
> -
> -	phydev = dev->phydev;
> -
> -	netif_carrier_off(dev);
> -
>  	err = register_netdev(dev);
>  	if (err) {
>  		dev_err(&pdev->dev, "Cannot register net device, aborting.\n");
>  		goto err_out_unregister_mdio;
>  	}
>  
> -	phy_attached_info(phydev);
> +	bp->phy_node = of_parse_phandle(bp->pdev->dev.of_node,
> +					"phy-handle", 0);
> +
> +	pdata = dev_get_platdata(&bp->pdev->dev);
> +	if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
> +		err = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin,
> +					"phy int");
> +		if (!err) {
> +			phy_irq = gpio_to_irq(pdata->phy_irq_pin);
> +			bp->phy_irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
> +		}
> +	}
> +
> +	netif_carrier_off(dev);
>  
>  	netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
>  		    macb_is_gem(bp) ? "GEM" : "MACB", macb_readl(bp, MID),
> @@ -3273,10 +3161,6 @@ static int macb_probe(struct platform_device *pdev)
>  	return 0;
>  
>  err_out_unregister_mdio:
> -	phy_disconnect(dev->phydev);
> -	mdiobus_unregister(bp->mii_bus);
> -	mdiobus_free(bp->mii_bus);
> -
>  	/* Shutdown the PHY if there is a GPIO reset */
>  	if (bp->reset_gpio)
>  		gpiod_set_value(bp->reset_gpio, 0);
> @@ -3304,9 +3188,6 @@ static int macb_remove(struct platform_device *pdev)
>  		bp = netdev_priv(dev);
>  		if (dev->phydev)
>  			phy_disconnect(dev->phydev);
> -		mdiobus_unregister(bp->mii_bus);
> -		dev->phydev = NULL;
> -		mdiobus_free(bp->mii_bus);
>  
>  		/* Shutdown the PHY if there is a GPIO reset */
>  		if (bp->reset_gpio)
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index d67adad..15e5c0f 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -874,6 +874,8 @@ struct macb {
>  	unsigned int		jumbo_max_len;
>  
>  	u32			wol;
> +	struct device_node *phy_node;
> +	int phy_irq;
>  };
>  
>  static inline bool macb_is_gem(struct macb *bp)
> diff --git a/drivers/net/ethernet/cadence/macb_mdio.c b/drivers/net/ethernet/cadence/macb_mdio.c
> new file mode 100644
> index 0000000..1277ca3
> --- /dev/null
> +++ b/drivers/net/ethernet/cadence/macb_mdio.c
> @@ -0,0 +1,266 @@
> +/*
> + * Cadence Macb mdio controller driver
> + *
> + * Copyright (C) 2014 - 2016 Xilinx, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it under
> + * the terms of the GNU General Public License version 2 as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/netdevice.h>
> +#include <linux/of_address.h>
> +#include <linux/of_mdio.h>
> +#include <linux/io.h>
> +#include <linux/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/ptp_clock_kernel.h>
> +#include "macb.h"
> +
> +struct macb_mdio_data {
> +	void __iomem *regs;
> +
> +	struct clk *pclk;
> +	struct clk *hclk;
> +};
> +
> +#define macb_mdio_reg_writel(bp, offset, value)	\
> +	writel_relaxed(value, bp->regs + offset)
> +#define macb_mdio_writel(bp, reg, value)	\
> +	macb_mdio_reg_writel(bp, MACB_##reg, value)
> +
> +#define macb_mdio_reg_readl(bp, offset)	readl_relaxed(bp->regs + offset)
> +#define macb_mdio_readl(bp, reg)	macb_mdio_reg_readl(bp, MACB_##reg)
> +
> +#define MACB_MDIO_TIMEOUT	1000
> +
> +static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> +{
> +	struct macb_mdio_data *bp = bus->priv;
> +	unsigned int timeout = MACB_MDIO_TIMEOUT;
> +	int value;
> +
> +	macb_mdio_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF) |
> +				   MACB_BF(RW, MACB_MAN_READ) |
> +				   MACB_BF(PHYA, mii_id) |
> +				   MACB_BF(REGA, regnum) |
> +				   MACB_BF(CODE, MACB_MAN_CODE)));
> +
> +	/* wait for end of transfer */
> +	while (!MACB_BFEXT(IDLE, macb_mdio_readl(bp, NSR)) && timeout) {
> +		cpu_relax();
> +		timeout--;
> +	}
> +
> +	if (!timeout)
> +		return -ETIMEDOUT;
> +
> +	value = MACB_BFEXT(DATA, macb_mdio_readl(bp, MAN));
> +
> +	return value;
> +}
> +
> +static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
> +			   u16 value)
> +{
> +	struct macb_mdio_data *bp = bus->priv;
> +	unsigned int timeout = MACB_MDIO_TIMEOUT;
> +
> +	macb_mdio_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF) |
> +				   MACB_BF(RW, MACB_MAN_WRITE) |
> +				   MACB_BF(PHYA, mii_id) |
> +				   MACB_BF(REGA, regnum) |
> +				   MACB_BF(CODE, MACB_MAN_CODE) |
> +				   MACB_BF(DATA, value)));
> +
> +	/* wait for end of transfer */
> +	while (!MACB_BFEXT(IDLE, macb_mdio_readl(bp, NSR)) && timeout) {
> +		cpu_relax();
> +		timeout--;
> +	}
> +
> +	if (!timeout)
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
> +static u32 gem_mdc_clk_div(struct macb_mdio_data *bp)
> +{
> +	u32 config;
> +	unsigned long pclk_hz = clk_get_rate(bp->pclk);
> +
> +	if (pclk_hz <= 20000000)
> +		config = GEM_BF(CLK, GEM_CLK_DIV8);
> +	else if (pclk_hz <= 40000000)
> +		config = GEM_BF(CLK, GEM_CLK_DIV16);
> +	else if (pclk_hz <= 80000000)
> +		config = GEM_BF(CLK, GEM_CLK_DIV32);
> +	else if (pclk_hz <= 120000000)
> +		config = GEM_BF(CLK, GEM_CLK_DIV48);
> +	else if (pclk_hz <= 160000000)
> +		config = GEM_BF(CLK, GEM_CLK_DIV64);
> +	else
> +		config = GEM_BF(CLK, GEM_CLK_DIV96);
> +
> +	return config;
> +}
> +
> +static int macb_mdio_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct mii_bus *bus;
> +	struct macb_mdio_data *bp;
> +	struct resource *res;
> +	int ret;
> +	u32 config, i;
> +
> +	bus = mdiobus_alloc_size(sizeof(*bp));
> +	if (!bus)
> +		return -ENOMEM;
> +
> +	bus->name = "macb_mii_bus";
> +	bus->read = &macb_mdio_read;
> +	bus->write = &macb_mdio_write;
> +	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(&pdev->dev));
> +	bus->parent = &pdev->dev;
> +	bus->irq = devm_kzalloc(&pdev->dev, sizeof(int) * PHY_MAX_ADDR,
> +				GFP_KERNEL);

This looks wrong, or at least old. It used to be a pointer to an array,
but it is now an actual array.

> +static const struct of_device_id macb_mdio_dt_ids[] = {
> +	{ .compatible = "cdns,macb-mdio" },
> +
> +};


I've not looked hard enough to know, but can you keep backwards
compatibility? Won't old device tree's assume the mdio bus is always
present? Now you need an explicit node otherwise there will not be an
mdio bus?

     Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
@ 2016-11-28  9:49 Harini Katakam
       [not found] ` <1480326554-6041-1-git-send-email-harinik-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 16+ messages in thread
From: Harini Katakam @ 2016-11-28  9:49 UTC (permalink / raw)
  To: nicolas.ferre, davem, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, boris.brezillon, alexandre.belloni,
	harinikatakamlinux
  Cc: netdev, linux-kernel, devicetree, harinik, michals,
	Punnaiah Choudary Kalluri

This patch is to add support for the hardware with multiple ethernet
MAC controllers and a single MDIO bus connected to multiple PHY devices.
MDIO lines are connected to any one of the ethernet MAC controllers and
all the PHY devices will be accessed using the PHY maintenance interface
in that MAC controller. This handling along with PHY functionality is
moved to macb_mdio.c

Signed-off-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
Signed-off-by: Harini Katakam <harinik@xilinx.com>
---
 drivers/net/ethernet/cadence/Makefile    |   2 +-
 drivers/net/ethernet/cadence/macb.c      | 169 +++-----------------
 drivers/net/ethernet/cadence/macb.h      |   2 +
 drivers/net/ethernet/cadence/macb_mdio.c | 266 +++++++++++++++++++++++++++++++
 4 files changed, 294 insertions(+), 145 deletions(-)
 create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c

diff --git a/drivers/net/ethernet/cadence/Makefile b/drivers/net/ethernet/cadence/Makefile
index 91f79b1..75c3d84 100644
--- a/drivers/net/ethernet/cadence/Makefile
+++ b/drivers/net/ethernet/cadence/Makefile
@@ -2,4 +2,4 @@
 # Makefile for the Atmel network device drivers.
 #
 
-obj-$(CONFIG_MACB) += macb.o
+obj-$(CONFIG_MACB) += macb.o macb_mdio.o
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 80ccfc4..ae2a797 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -232,45 +232,6 @@ static void macb_get_hwaddr(struct macb *bp)
 	eth_hw_addr_random(bp->dev);
 }
 
-static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
-{
-	struct macb *bp = bus->priv;
-	int value;
-
-	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
-			      | MACB_BF(RW, MACB_MAN_READ)
-			      | MACB_BF(PHYA, mii_id)
-			      | MACB_BF(REGA, regnum)
-			      | MACB_BF(CODE, MACB_MAN_CODE)));
-
-	/* wait for end of transfer */
-	while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
-		cpu_relax();
-
-	value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
-
-	return value;
-}
-
-static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
-			   u16 value)
-{
-	struct macb *bp = bus->priv;
-
-	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
-			      | MACB_BF(RW, MACB_MAN_WRITE)
-			      | MACB_BF(PHYA, mii_id)
-			      | MACB_BF(REGA, regnum)
-			      | MACB_BF(CODE, MACB_MAN_CODE)
-			      | MACB_BF(DATA, value)));
-
-	/* wait for end of transfer */
-	while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
-		cpu_relax();
-
-	return 0;
-}
-
 /**
  * macb_set_tx_clk() - Set a clock to a new frequency
  * @clk		Pointer to the clock to change
@@ -385,27 +346,19 @@ static void macb_handle_link_change(struct net_device *dev)
 static int macb_mii_probe(struct net_device *dev)
 {
 	struct macb *bp = netdev_priv(dev);
-	struct macb_platform_data *pdata;
 	struct phy_device *phydev;
-	int phy_irq;
 	int ret;
 
-	phydev = phy_find_first(bp->mii_bus);
+	if (dev->phydev)
+		return 0;
+
+	phydev = of_phy_find_device(bp->phy_node);
 	if (!phydev) {
 		netdev_err(dev, "no PHY found\n");
 		return -ENXIO;
 	}
-
-	pdata = dev_get_platdata(&bp->pdev->dev);
-	if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
-		ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin,
-					"phy int");
-		if (!ret) {
-			phy_irq = gpio_to_irq(pdata->phy_irq_pin);
-			phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
-		}
-	}
-
+	if (bp->phy_irq)
+		phydev->irq = bp->phy_irq;
 	/* attach the mac to the phy */
 	ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
 				 bp->phy_interface);
@@ -429,80 +382,9 @@ static int macb_mii_probe(struct net_device *dev)
 	bp->speed = 0;
 	bp->duplex = -1;
 
-	return 0;
-}
-
-static int macb_mii_init(struct macb *bp)
-{
-	struct macb_platform_data *pdata;
-	struct device_node *np;
-	int err = -ENXIO, i;
-
-	/* Enable management port */
-	macb_writel(bp, NCR, MACB_BIT(MPE));
-
-	bp->mii_bus = mdiobus_alloc();
-	if (!bp->mii_bus) {
-		err = -ENOMEM;
-		goto err_out;
-	}
-
-	bp->mii_bus->name = "MACB_mii_bus";
-	bp->mii_bus->read = &macb_mdio_read;
-	bp->mii_bus->write = &macb_mdio_write;
-	snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
-		 bp->pdev->name, bp->pdev->id);
-	bp->mii_bus->priv = bp;
-	bp->mii_bus->parent = &bp->pdev->dev;
-	pdata = dev_get_platdata(&bp->pdev->dev);
-
-	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
-
-	np = bp->pdev->dev.of_node;
-	if (np) {
-		/* try dt phy registration */
-		err = of_mdiobus_register(bp->mii_bus, np);
-
-		/* fallback to standard phy registration if no phy were
-		 * found during dt phy registration
-		 */
-		if (!err && !phy_find_first(bp->mii_bus)) {
-			for (i = 0; i < PHY_MAX_ADDR; i++) {
-				struct phy_device *phydev;
-
-				phydev = mdiobus_scan(bp->mii_bus, i);
-				if (IS_ERR(phydev) &&
-				    PTR_ERR(phydev) != -ENODEV) {
-					err = PTR_ERR(phydev);
-					break;
-				}
-			}
-
-			if (err)
-				goto err_out_unregister_bus;
-		}
-	} else {
-		if (pdata)
-			bp->mii_bus->phy_mask = pdata->phy_mask;
-
-		err = mdiobus_register(bp->mii_bus);
-	}
-
-	if (err)
-		goto err_out_free_mdiobus;
-
-	err = macb_mii_probe(bp->dev);
-	if (err)
-		goto err_out_unregister_bus;
+	phy_attached_info(phydev);
 
 	return 0;
-
-err_out_unregister_bus:
-	mdiobus_unregister(bp->mii_bus);
-err_out_free_mdiobus:
-	mdiobus_free(bp->mii_bus);
-err_out:
-	return err;
 }
 
 static void macb_update_stats(struct macb *bp)
@@ -2060,7 +1942,8 @@ static int macb_open(struct net_device *dev)
 	netif_carrier_off(dev);
 
 	/* if the phy is not yet register, retry later*/
-	if (!dev->phydev)
+	err = macb_mii_probe(dev);
+	if (err)
 		return -EAGAIN;
 
 	/* RX buffers initialization */
@@ -3122,16 +3005,16 @@ static int macb_probe(struct platform_device *pdev)
 	unsigned int queue_mask, num_queues;
 	struct macb_platform_data *pdata;
 	bool native_io;
-	struct phy_device *phydev;
 	struct net_device *dev;
 	struct resource *regs;
 	void __iomem *mem;
 	const char *mac;
 	struct macb *bp;
+	int phy_irq;
 	int err;
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	mem = devm_ioremap_resource(&pdev->dev, regs);
+	mem = devm_ioremap(&pdev->dev, regs->start, resource_size(regs));
 	if (IS_ERR(mem))
 		return PTR_ERR(mem);
 
@@ -3250,21 +3133,26 @@ static int macb_probe(struct platform_device *pdev)
 	if (err)
 		goto err_out_free_netdev;
 
-	err = macb_mii_init(bp);
-	if (err)
-		goto err_out_free_netdev;
-
-	phydev = dev->phydev;
-
-	netif_carrier_off(dev);
-
 	err = register_netdev(dev);
 	if (err) {
 		dev_err(&pdev->dev, "Cannot register net device, aborting.\n");
 		goto err_out_unregister_mdio;
 	}
 
-	phy_attached_info(phydev);
+	bp->phy_node = of_parse_phandle(bp->pdev->dev.of_node,
+					"phy-handle", 0);
+
+	pdata = dev_get_platdata(&bp->pdev->dev);
+	if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
+		err = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin,
+					"phy int");
+		if (!err) {
+			phy_irq = gpio_to_irq(pdata->phy_irq_pin);
+			bp->phy_irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
+		}
+	}
+
+	netif_carrier_off(dev);
 
 	netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
 		    macb_is_gem(bp) ? "GEM" : "MACB", macb_readl(bp, MID),
@@ -3273,10 +3161,6 @@ static int macb_probe(struct platform_device *pdev)
 	return 0;
 
 err_out_unregister_mdio:
-	phy_disconnect(dev->phydev);
-	mdiobus_unregister(bp->mii_bus);
-	mdiobus_free(bp->mii_bus);
-
 	/* Shutdown the PHY if there is a GPIO reset */
 	if (bp->reset_gpio)
 		gpiod_set_value(bp->reset_gpio, 0);
@@ -3304,9 +3188,6 @@ static int macb_remove(struct platform_device *pdev)
 		bp = netdev_priv(dev);
 		if (dev->phydev)
 			phy_disconnect(dev->phydev);
-		mdiobus_unregister(bp->mii_bus);
-		dev->phydev = NULL;
-		mdiobus_free(bp->mii_bus);
 
 		/* Shutdown the PHY if there is a GPIO reset */
 		if (bp->reset_gpio)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index d67adad..15e5c0f 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -874,6 +874,8 @@ struct macb {
 	unsigned int		jumbo_max_len;
 
 	u32			wol;
+	struct device_node *phy_node;
+	int phy_irq;
 };
 
 static inline bool macb_is_gem(struct macb *bp)
diff --git a/drivers/net/ethernet/cadence/macb_mdio.c b/drivers/net/ethernet/cadence/macb_mdio.c
new file mode 100644
index 0000000..1277ca3
--- /dev/null
+++ b/drivers/net/ethernet/cadence/macb_mdio.c
@@ -0,0 +1,266 @@
+/*
+ * Cadence Macb mdio controller driver
+ *
+ * Copyright (C) 2014 - 2016 Xilinx, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License version 2 as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/netdevice.h>
+#include <linux/of_address.h>
+#include <linux/of_mdio.h>
+#include <linux/io.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include <linux/ptp_clock_kernel.h>
+#include "macb.h"
+
+struct macb_mdio_data {
+	void __iomem *regs;
+
+	struct clk *pclk;
+	struct clk *hclk;
+};
+
+#define macb_mdio_reg_writel(bp, offset, value)	\
+	writel_relaxed(value, bp->regs + offset)
+#define macb_mdio_writel(bp, reg, value)	\
+	macb_mdio_reg_writel(bp, MACB_##reg, value)
+
+#define macb_mdio_reg_readl(bp, offset)	readl_relaxed(bp->regs + offset)
+#define macb_mdio_readl(bp, reg)	macb_mdio_reg_readl(bp, MACB_##reg)
+
+#define MACB_MDIO_TIMEOUT	1000
+
+static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
+{
+	struct macb_mdio_data *bp = bus->priv;
+	unsigned int timeout = MACB_MDIO_TIMEOUT;
+	int value;
+
+	macb_mdio_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF) |
+				   MACB_BF(RW, MACB_MAN_READ) |
+				   MACB_BF(PHYA, mii_id) |
+				   MACB_BF(REGA, regnum) |
+				   MACB_BF(CODE, MACB_MAN_CODE)));
+
+	/* wait for end of transfer */
+	while (!MACB_BFEXT(IDLE, macb_mdio_readl(bp, NSR)) && timeout) {
+		cpu_relax();
+		timeout--;
+	}
+
+	if (!timeout)
+		return -ETIMEDOUT;
+
+	value = MACB_BFEXT(DATA, macb_mdio_readl(bp, MAN));
+
+	return value;
+}
+
+static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
+			   u16 value)
+{
+	struct macb_mdio_data *bp = bus->priv;
+	unsigned int timeout = MACB_MDIO_TIMEOUT;
+
+	macb_mdio_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF) |
+				   MACB_BF(RW, MACB_MAN_WRITE) |
+				   MACB_BF(PHYA, mii_id) |
+				   MACB_BF(REGA, regnum) |
+				   MACB_BF(CODE, MACB_MAN_CODE) |
+				   MACB_BF(DATA, value)));
+
+	/* wait for end of transfer */
+	while (!MACB_BFEXT(IDLE, macb_mdio_readl(bp, NSR)) && timeout) {
+		cpu_relax();
+		timeout--;
+	}
+
+	if (!timeout)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static u32 gem_mdc_clk_div(struct macb_mdio_data *bp)
+{
+	u32 config;
+	unsigned long pclk_hz = clk_get_rate(bp->pclk);
+
+	if (pclk_hz <= 20000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV8);
+	else if (pclk_hz <= 40000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV16);
+	else if (pclk_hz <= 80000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV32);
+	else if (pclk_hz <= 120000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV48);
+	else if (pclk_hz <= 160000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV64);
+	else
+		config = GEM_BF(CLK, GEM_CLK_DIV96);
+
+	return config;
+}
+
+static int macb_mdio_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct mii_bus *bus;
+	struct macb_mdio_data *bp;
+	struct resource *res;
+	int ret;
+	u32 config, i;
+
+	bus = mdiobus_alloc_size(sizeof(*bp));
+	if (!bus)
+		return -ENOMEM;
+
+	bus->name = "macb_mii_bus";
+	bus->read = &macb_mdio_read;
+	bus->write = &macb_mdio_write;
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(&pdev->dev));
+	bus->parent = &pdev->dev;
+	bus->irq = devm_kzalloc(&pdev->dev, sizeof(int) * PHY_MAX_ADDR,
+				GFP_KERNEL);
+	if (!bus->irq) {
+		ret = -ENOMEM;
+		goto err_out_free_mdiobus;
+	}
+
+	bp = bus->priv;
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	bp->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (IS_ERR(bp->regs)) {
+		ret = PTR_ERR(bp->regs);
+		goto err_out_free_mdiobus;
+	}
+
+	bp->pclk = devm_clk_get(&pdev->dev, "pclk");
+	if (IS_ERR(bp->pclk)) {
+		ret = PTR_ERR(bp->pclk);
+		dev_err(&pdev->dev, "failed to get macb_clk (%u)\n", ret);
+		goto err_out_free_mdiobus;
+	}
+
+	bp->hclk = devm_clk_get(&pdev->dev, "hclk");
+	if (IS_ERR(bp->hclk)) {
+		ret = PTR_ERR(bp->hclk);
+		dev_err(&pdev->dev, "failed to get hclk (%u)\n", ret);
+		goto err_out_free_mdiobus;
+	}
+
+	ret = clk_prepare_enable(bp->pclk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable pclk (%u)\n", ret);
+		goto err_out_free_mdiobus;
+	}
+
+	ret = clk_prepare_enable(bp->hclk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable hclk (%u)\n", ret);
+		goto err_disable_pclk;
+	}
+
+	platform_set_drvdata(pdev, bus);
+
+	/* Enable management port */
+	config = macb_mdio_readl(bp, NCR);
+	config |= MACB_BIT(MPE);
+	macb_mdio_writel(bp, NCR, config);
+	config = gem_mdc_clk_div(bp);
+	macb_mdio_writel(bp, NCFGR, config);
+
+	np = pdev->dev.of_node;
+	if (np) {
+		/* try dt phy registration */
+		ret = of_mdiobus_register(bus, np);
+
+		/* Fallback to standard phy registration if no phy were
+		 * found during dt phy registration
+		 */
+		if (!ret && !phy_find_first(bus)) {
+			for (i = 0; i < PHY_MAX_ADDR; i++) {
+				struct phy_device *phydev;
+
+				phydev = mdiobus_scan(bus, i);
+				if (IS_ERR(phydev) &&
+				    PTR_ERR(phydev) != -ENODEV) {
+					ret = PTR_ERR(phydev);
+					break;
+				}
+			}
+
+			if (ret)
+				goto err_out_unregister_bus;
+		}
+	} else {
+		for (i = 0; i < PHY_MAX_ADDR; i++)
+			bus->irq[i] = PHY_POLL;
+
+		ret = of_mdiobus_register(bus, np);
+	}
+
+	if (ret)
+		goto err_out_free_mdio_irq;
+
+	return 0;
+
+err_out_unregister_bus:
+	mdiobus_unregister(bus);
+err_out_free_mdio_irq:
+	kfree(bus->irq);
+err_disable_pclk:
+	clk_disable_unprepare(bp->pclk);
+	clk_disable_unprepare(bp->hclk);
+err_out_free_mdiobus:
+	mdiobus_free(bus);
+	return ret;
+}
+
+static int macb_mdio_remove(struct platform_device *pdev)
+{
+	struct mii_bus *bus = platform_get_drvdata(pdev);
+	struct macb_mdio_data *bp = bus->priv;
+	u32 config;
+
+	/* Disable management port */
+	config = macb_mdio_readl(bp, NCR);
+	config &= ~MACB_BIT(MPE);
+	macb_mdio_writel(bp, NCR, config);
+	mdiobus_unregister(bus);
+	clk_disable_unprepare(bp->hclk);
+	clk_disable_unprepare(bp->pclk);
+	mdiobus_free(bus);
+
+	return 0;
+}
+
+static const struct of_device_id macb_mdio_dt_ids[] = {
+	{ .compatible = "cdns,macb-mdio" },
+
+};
+MODULE_DEVICE_TABLE(of, macb_mdio_dt_ids);
+
+static struct platform_driver macb_mdio_driver = {
+	.probe = macb_mdio_probe,
+	.remove = macb_mdio_remove,
+	.driver = {
+		.name = "macb-mdio",
+		.of_match_table = macb_mdio_dt_ids,
+	},
+};
+
+module_platform_driver(macb_mdio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Cadence MACB MDIO driver");
+MODULE_AUTHOR("Xilinx");
-- 
2.7.4

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

* [RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
@ 2016-05-13  9:26 Harini Katakam
  0 siblings, 0 replies; 16+ messages in thread
From: Harini Katakam @ 2016-05-13  9:26 UTC (permalink / raw)
  To: nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ,
	harinikatakamlinux-Re5JQEeQqe8AvxtiuMwx3w
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	harinik-gjFFaj9aHVfQT0dZR+AlfA, punnaia-gjFFaj9aHVfQT0dZR+AlfA,
	michals-gjFFaj9aHVfQT0dZR+AlfA

This patch is to add support for the hardware with multiple ethernet
MAC controllers and a single MDIO bus connected to multiple PHY devices.
MDIO lines are connected to any one of the ethernet MAC controllers and
all the PHY devices will be accessed using the PHY maintenance interface
in that MAC controller. This handling along with PHY functionality is
moved to macb_mdio.c

Signed-off-by: Punnaiah Choudary Kalluri <punnaia-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Harini Katakam <harinik-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
 drivers/net/ethernet/cadence/Makefile    |    2 +-
 drivers/net/ethernet/cadence/macb.c      |  183 ++++----------------
 drivers/net/ethernet/cadence/macb.h      |    2 +
 drivers/net/ethernet/cadence/macb_mdio.c |  266 ++++++++++++++++++++++++++++++
 4 files changed, 302 insertions(+), 151 deletions(-)
 create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c

diff --git a/drivers/net/ethernet/cadence/Makefile b/drivers/net/ethernet/cadence/Makefile
index 91f79b1..75c3d84 100644
--- a/drivers/net/ethernet/cadence/Makefile
+++ b/drivers/net/ethernet/cadence/Makefile
@@ -2,4 +2,4 @@
 # Makefile for the Atmel network device drivers.
 #
 
-obj-$(CONFIG_MACB) += macb.o
+obj-$(CONFIG_MACB) += macb.o macb_mdio.o
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index cb07d95..6e8ff4e 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -218,45 +218,6 @@ static void macb_get_hwaddr(struct macb *bp)
 	eth_hw_addr_random(bp->dev);
 }
 
-static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
-{
-	struct macb *bp = bus->priv;
-	int value;
-
-	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
-			      | MACB_BF(RW, MACB_MAN_READ)
-			      | MACB_BF(PHYA, mii_id)
-			      | MACB_BF(REGA, regnum)
-			      | MACB_BF(CODE, MACB_MAN_CODE)));
-
-	/* wait for end of transfer */
-	while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
-		cpu_relax();
-
-	value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
-
-	return value;
-}
-
-static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
-			   u16 value)
-{
-	struct macb *bp = bus->priv;
-
-	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
-			      | MACB_BF(RW, MACB_MAN_WRITE)
-			      | MACB_BF(PHYA, mii_id)
-			      | MACB_BF(REGA, regnum)
-			      | MACB_BF(CODE, MACB_MAN_CODE)
-			      | MACB_BF(DATA, value)));
-
-	/* wait for end of transfer */
-	while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
-		cpu_relax();
-
-	return 0;
-}
-
 /**
  * macb_set_tx_clk() - Set a clock to a new frequency
  * @clk		Pointer to the clock to change
@@ -371,27 +332,19 @@ static void macb_handle_link_change(struct net_device *dev)
 static int macb_mii_probe(struct net_device *dev)
 {
 	struct macb *bp = netdev_priv(dev);
-	struct macb_platform_data *pdata;
 	struct phy_device *phydev;
-	int phy_irq;
 	int ret;
 
-	phydev = phy_find_first(bp->mii_bus);
+	if (bp->phy_dev)
+		return 0;
+
+	phydev = of_phy_find_device(bp->phy_node);
 	if (!phydev) {
 		netdev_err(dev, "no PHY found\n");
 		return -ENXIO;
 	}
-
-	pdata = dev_get_platdata(&bp->pdev->dev);
-	if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
-		ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin,
-					"phy int");
-		if (!ret) {
-			phy_irq = gpio_to_irq(pdata->phy_irq_pin);
-			phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
-		}
-	}
-
+	if (bp->phy_irq)
+		phydev->irq = bp->phy_irq;
 	/* attach the mac to the phy */
 	ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
 				 bp->phy_interface);
@@ -416,80 +369,9 @@ static int macb_mii_probe(struct net_device *dev)
 	bp->duplex = -1;
 	bp->phy_dev = phydev;
 
-	return 0;
-}
-
-static int macb_mii_init(struct macb *bp)
-{
-	struct macb_platform_data *pdata;
-	struct device_node *np;
-	int err = -ENXIO, i;
-
-	/* Enable management port */
-	macb_writel(bp, NCR, MACB_BIT(MPE));
-
-	bp->mii_bus = mdiobus_alloc();
-	if (!bp->mii_bus) {
-		err = -ENOMEM;
-		goto err_out;
-	}
-
-	bp->mii_bus->name = "MACB_mii_bus";
-	bp->mii_bus->read = &macb_mdio_read;
-	bp->mii_bus->write = &macb_mdio_write;
-	snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
-		 bp->pdev->name, bp->pdev->id);
-	bp->mii_bus->priv = bp;
-	bp->mii_bus->parent = &bp->pdev->dev;
-	pdata = dev_get_platdata(&bp->pdev->dev);
-
-	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
-
-	np = bp->pdev->dev.of_node;
-	if (np) {
-		/* try dt phy registration */
-		err = of_mdiobus_register(bp->mii_bus, np);
-
-		/* fallback to standard phy registration if no phy were
-		 * found during dt phy registration
-		 */
-		if (!err && !phy_find_first(bp->mii_bus)) {
-			for (i = 0; i < PHY_MAX_ADDR; i++) {
-				struct phy_device *phydev;
-
-				phydev = mdiobus_scan(bp->mii_bus, i);
-				if (IS_ERR(phydev) &&
-				    PTR_ERR(phydev) != -ENODEV) {
-					err = PTR_ERR(phydev);
-					break;
-				}
-			}
-
-			if (err)
-				goto err_out_unregister_bus;
-		}
-	} else {
-		if (pdata)
-			bp->mii_bus->phy_mask = pdata->phy_mask;
-
-		err = mdiobus_register(bp->mii_bus);
-	}
-
-	if (err)
-		goto err_out_free_mdiobus;
-
-	err = macb_mii_probe(bp->dev);
-	if (err)
-		goto err_out_unregister_bus;
+	phy_attached_info(phydev);
 
 	return 0;
-
-err_out_unregister_bus:
-	mdiobus_unregister(bp->mii_bus);
-err_out_free_mdiobus:
-	mdiobus_free(bp->mii_bus);
-err_out:
-	return err;
 }
 
 static void macb_update_stats(struct macb *bp)
@@ -1582,15 +1464,18 @@ static void macb_init_rings(struct macb *bp)
 static void macb_reset_hw(struct macb *bp)
 {
 	struct macb_queue *queue;
-	unsigned int q;
+	unsigned int q, ctrl;
 
 	/* Disable RX and TX (XXX: Should we halt the transmission
 	 * more gracefully?)
 	 */
-	macb_writel(bp, NCR, 0);
+	ctrl = macb_readl(bp, NCR);
+	ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
+	macb_writel(bp, NCR, ctrl);
 
 	/* Clear the stats registers (XXX: Update stats first?) */
-	macb_writel(bp, NCR, MACB_BIT(CLRSTAT));
+	ctrl |= MACB_BIT(CLRSTAT);
+	macb_writel(bp, NCR, ctrl);
 
 	/* Clear all status flags */
 	macb_writel(bp, TSR, -1);
@@ -1886,7 +1771,8 @@ static int macb_open(struct net_device *dev)
 	netif_carrier_off(dev);
 
 	/* if the phy is not yet register, retry later*/
-	if (!bp->phy_dev)
+	err = macb_mii_probe(dev);
+	if (err)
 		return -EAGAIN;
 
 	/* RX buffers initialization */
@@ -2891,16 +2777,16 @@ static int macb_probe(struct platform_device *pdev)
 	unsigned int queue_mask, num_queues;
 	struct macb_platform_data *pdata;
 	bool native_io;
-	struct phy_device *phydev;
 	struct net_device *dev;
 	struct resource *regs;
 	void __iomem *mem;
 	const char *mac;
 	struct macb *bp;
+	int phy_irq;
 	int err;
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	mem = devm_ioremap_resource(&pdev->dev, regs);
+	mem = devm_ioremap(&pdev->dev, regs->start, resource_size(regs));
 	if (IS_ERR(mem))
 		return PTR_ERR(mem);
 
@@ -3006,21 +2892,26 @@ static int macb_probe(struct platform_device *pdev)
 	if (err)
 		goto err_out_free_netdev;
 
-	err = macb_mii_init(bp);
-	if (err)
-		goto err_out_free_netdev;
-
-	phydev = bp->phy_dev;
-
-	netif_carrier_off(dev);
-
 	err = register_netdev(dev);
 	if (err) {
 		dev_err(&pdev->dev, "Cannot register net device, aborting.\n");
-		goto err_out_unregister_mdio;
+		goto err_out_free_netdev;
 	}
 
-	phy_attached_info(phydev);
+	bp->phy_node = of_parse_phandle(bp->pdev->dev.of_node,
+					"phy-handle", 0);
+
+	pdata = dev_get_platdata(&bp->pdev->dev);
+	if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
+		err = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin,
+					"phy int");
+		if (!err) {
+			phy_irq = gpio_to_irq(pdata->phy_irq_pin);
+			bp->phy_irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
+		}
+	}
+
+	netif_carrier_off(dev);
 
 	netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
 		    macb_is_gem(bp) ? "GEM" : "MACB", macb_readl(bp, MID),
@@ -3028,16 +2919,10 @@ static int macb_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_out_unregister_mdio:
-	phy_disconnect(bp->phy_dev);
-	mdiobus_unregister(bp->mii_bus);
-	mdiobus_free(bp->mii_bus);
-
+err_out_free_netdev:
 	/* Shutdown the PHY if there is a GPIO reset */
 	if (bp->reset_gpio)
 		gpiod_set_value(bp->reset_gpio, 0);
-
-err_out_free_netdev:
 	free_netdev(dev);
 
 err_disable_clocks:
@@ -3059,8 +2944,6 @@ static int macb_remove(struct platform_device *pdev)
 		bp = netdev_priv(dev);
 		if (bp->phy_dev)
 			phy_disconnect(bp->phy_dev);
-		mdiobus_unregister(bp->mii_bus);
-		mdiobus_free(bp->mii_bus);
 
 		/* Shutdown the PHY if there is a GPIO reset */
 		if (bp->reset_gpio)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 8a13824..a63b215 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -846,6 +846,8 @@ struct macb {
 	unsigned int		jumbo_max_len;
 
 	u32			wol;
+	struct device_node *phy_node;
+	int phy_irq;
 };
 
 static inline bool macb_is_gem(struct macb *bp)
diff --git a/drivers/net/ethernet/cadence/macb_mdio.c b/drivers/net/ethernet/cadence/macb_mdio.c
new file mode 100644
index 0000000..1277ca3
--- /dev/null
+++ b/drivers/net/ethernet/cadence/macb_mdio.c
@@ -0,0 +1,266 @@
+/*
+ * Cadence Macb mdio controller driver
+ *
+ * Copyright (C) 2014 - 2016 Xilinx, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License version 2 as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/netdevice.h>
+#include <linux/of_address.h>
+#include <linux/of_mdio.h>
+#include <linux/io.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include <linux/ptp_clock_kernel.h>
+#include "macb.h"
+
+struct macb_mdio_data {
+	void __iomem *regs;
+
+	struct clk *pclk;
+	struct clk *hclk;
+};
+
+#define macb_mdio_reg_writel(bp, offset, value)	\
+	writel_relaxed(value, bp->regs + offset)
+#define macb_mdio_writel(bp, reg, value)	\
+	macb_mdio_reg_writel(bp, MACB_##reg, value)
+
+#define macb_mdio_reg_readl(bp, offset)	readl_relaxed(bp->regs + offset)
+#define macb_mdio_readl(bp, reg)	macb_mdio_reg_readl(bp, MACB_##reg)
+
+#define MACB_MDIO_TIMEOUT	1000
+
+static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
+{
+	struct macb_mdio_data *bp = bus->priv;
+	unsigned int timeout = MACB_MDIO_TIMEOUT;
+	int value;
+
+	macb_mdio_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF) |
+				   MACB_BF(RW, MACB_MAN_READ) |
+				   MACB_BF(PHYA, mii_id) |
+				   MACB_BF(REGA, regnum) |
+				   MACB_BF(CODE, MACB_MAN_CODE)));
+
+	/* wait for end of transfer */
+	while (!MACB_BFEXT(IDLE, macb_mdio_readl(bp, NSR)) && timeout) {
+		cpu_relax();
+		timeout--;
+	}
+
+	if (!timeout)
+		return -ETIMEDOUT;
+
+	value = MACB_BFEXT(DATA, macb_mdio_readl(bp, MAN));
+
+	return value;
+}
+
+static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
+			   u16 value)
+{
+	struct macb_mdio_data *bp = bus->priv;
+	unsigned int timeout = MACB_MDIO_TIMEOUT;
+
+	macb_mdio_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF) |
+				   MACB_BF(RW, MACB_MAN_WRITE) |
+				   MACB_BF(PHYA, mii_id) |
+				   MACB_BF(REGA, regnum) |
+				   MACB_BF(CODE, MACB_MAN_CODE) |
+				   MACB_BF(DATA, value)));
+
+	/* wait for end of transfer */
+	while (!MACB_BFEXT(IDLE, macb_mdio_readl(bp, NSR)) && timeout) {
+		cpu_relax();
+		timeout--;
+	}
+
+	if (!timeout)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static u32 gem_mdc_clk_div(struct macb_mdio_data *bp)
+{
+	u32 config;
+	unsigned long pclk_hz = clk_get_rate(bp->pclk);
+
+	if (pclk_hz <= 20000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV8);
+	else if (pclk_hz <= 40000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV16);
+	else if (pclk_hz <= 80000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV32);
+	else if (pclk_hz <= 120000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV48);
+	else if (pclk_hz <= 160000000)
+		config = GEM_BF(CLK, GEM_CLK_DIV64);
+	else
+		config = GEM_BF(CLK, GEM_CLK_DIV96);
+
+	return config;
+}
+
+static int macb_mdio_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct mii_bus *bus;
+	struct macb_mdio_data *bp;
+	struct resource *res;
+	int ret;
+	u32 config, i;
+
+	bus = mdiobus_alloc_size(sizeof(*bp));
+	if (!bus)
+		return -ENOMEM;
+
+	bus->name = "macb_mii_bus";
+	bus->read = &macb_mdio_read;
+	bus->write = &macb_mdio_write;
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(&pdev->dev));
+	bus->parent = &pdev->dev;
+	bus->irq = devm_kzalloc(&pdev->dev, sizeof(int) * PHY_MAX_ADDR,
+				GFP_KERNEL);
+	if (!bus->irq) {
+		ret = -ENOMEM;
+		goto err_out_free_mdiobus;
+	}
+
+	bp = bus->priv;
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	bp->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (IS_ERR(bp->regs)) {
+		ret = PTR_ERR(bp->regs);
+		goto err_out_free_mdiobus;
+	}
+
+	bp->pclk = devm_clk_get(&pdev->dev, "pclk");
+	if (IS_ERR(bp->pclk)) {
+		ret = PTR_ERR(bp->pclk);
+		dev_err(&pdev->dev, "failed to get macb_clk (%u)\n", ret);
+		goto err_out_free_mdiobus;
+	}
+
+	bp->hclk = devm_clk_get(&pdev->dev, "hclk");
+	if (IS_ERR(bp->hclk)) {
+		ret = PTR_ERR(bp->hclk);
+		dev_err(&pdev->dev, "failed to get hclk (%u)\n", ret);
+		goto err_out_free_mdiobus;
+	}
+
+	ret = clk_prepare_enable(bp->pclk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable pclk (%u)\n", ret);
+		goto err_out_free_mdiobus;
+	}
+
+	ret = clk_prepare_enable(bp->hclk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable hclk (%u)\n", ret);
+		goto err_disable_pclk;
+	}
+
+	platform_set_drvdata(pdev, bus);
+
+	/* Enable management port */
+	config = macb_mdio_readl(bp, NCR);
+	config |= MACB_BIT(MPE);
+	macb_mdio_writel(bp, NCR, config);
+	config = gem_mdc_clk_div(bp);
+	macb_mdio_writel(bp, NCFGR, config);
+
+	np = pdev->dev.of_node;
+	if (np) {
+		/* try dt phy registration */
+		ret = of_mdiobus_register(bus, np);
+
+		/* Fallback to standard phy registration if no phy were
+		 * found during dt phy registration
+		 */
+		if (!ret && !phy_find_first(bus)) {
+			for (i = 0; i < PHY_MAX_ADDR; i++) {
+				struct phy_device *phydev;
+
+				phydev = mdiobus_scan(bus, i);
+				if (IS_ERR(phydev) &&
+				    PTR_ERR(phydev) != -ENODEV) {
+					ret = PTR_ERR(phydev);
+					break;
+				}
+			}
+
+			if (ret)
+				goto err_out_unregister_bus;
+		}
+	} else {
+		for (i = 0; i < PHY_MAX_ADDR; i++)
+			bus->irq[i] = PHY_POLL;
+
+		ret = of_mdiobus_register(bus, np);
+	}
+
+	if (ret)
+		goto err_out_free_mdio_irq;
+
+	return 0;
+
+err_out_unregister_bus:
+	mdiobus_unregister(bus);
+err_out_free_mdio_irq:
+	kfree(bus->irq);
+err_disable_pclk:
+	clk_disable_unprepare(bp->pclk);
+	clk_disable_unprepare(bp->hclk);
+err_out_free_mdiobus:
+	mdiobus_free(bus);
+	return ret;
+}
+
+static int macb_mdio_remove(struct platform_device *pdev)
+{
+	struct mii_bus *bus = platform_get_drvdata(pdev);
+	struct macb_mdio_data *bp = bus->priv;
+	u32 config;
+
+	/* Disable management port */
+	config = macb_mdio_readl(bp, NCR);
+	config &= ~MACB_BIT(MPE);
+	macb_mdio_writel(bp, NCR, config);
+	mdiobus_unregister(bus);
+	clk_disable_unprepare(bp->hclk);
+	clk_disable_unprepare(bp->pclk);
+	mdiobus_free(bus);
+
+	return 0;
+}
+
+static const struct of_device_id macb_mdio_dt_ids[] = {
+	{ .compatible = "cdns,macb-mdio" },
+
+};
+MODULE_DEVICE_TABLE(of, macb_mdio_dt_ids);
+
+static struct platform_driver macb_mdio_driver = {
+	.probe = macb_mdio_probe,
+	.remove = macb_mdio_remove,
+	.driver = {
+		.name = "macb-mdio",
+		.of_match_table = macb_mdio_dt_ids,
+	},
+};
+
+module_platform_driver(macb_mdio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Cadence MACB MDIO driver");
+MODULE_AUTHOR("Xilinx");
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2016-11-29  4:11 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-13  4:48 [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices Punnaiah Choudary Kalluri
2015-07-13  4:48 ` [RFC PATCH 1/2] " Punnaiah Choudary Kalluri
2015-07-13  4:48 ` [RFC PATCH 2/2] net: macb: Add support for single mac managing more than one phy Punnaiah Choudary Kalluri
2015-07-13 18:43 ` [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices Florian Fainelli
2015-07-14  3:02   ` punnaiah choudary kalluri
2015-07-20 13:30 ` Michal Simek
2015-07-20 16:23   ` Andrew Lunn
2015-07-27  7:37   ` Nicolas Ferre
2015-07-28  3:34     ` Punnaiah Choudary Kalluri
2015-07-31 21:53       ` Nathan Sullivan
2015-08-03  6:01         ` Michal Simek
2015-07-31 21:58       ` Nathan Sullivan
2016-05-13  9:26 [RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices Harini Katakam
2016-11-28  9:49 Harini Katakam
     [not found] ` <1480326554-6041-1-git-send-email-harinik-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
2016-11-28 16:33   ` Andrew Lunn
2016-11-29  4:11     ` Harini Katakam

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