[PATCH 0/5] mv643xx_eth: use mvmdio MDIO bus driver

[PATCH 0/5] mv643xx_eth: use mvmdio MDIO bus driver

[Florian Fainelli]

Hi all,

This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
instead of rolling its own implementation. As a result, all users of this
mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
The mvmdio driver is also updated to support an interrupt line which reports
SMI error/completion, and to allow traditionnal platform device registration
instead of just device tree.

David, I think it makes sense for you to merge all of this, since we do
not want the architecture files to be desynchronized from the mv643xx_eth to
avoid runtime breakage. The potential for merge conflicts should be very small.

You can probably safely merge patches 1 to 4 if Thomas agrees, and we will
see what kind of feeback I get on patch 5.

Florian Fainelli (5):
  net: mvmdio: unmap base register address at driver removal
  net: mvmdio: rename base register cookie from smireg to regs
  net: mvmdio: enhance driver to support SMI error/done interrupts
  net: mvmdio: allow Device Tree and platform device to coexist
  mv643xx_eth: convert to use the Marvell Orion MDIO driver

 .../devicetree/bindings/net/marvell-orion-mdio.txt |    3 +
 arch/arm/plat-orion/common.c                       |   84 +++++++--
 arch/powerpc/platforms/chrp/pegasos_eth.c          |   20 +++
 arch/powerpc/sysdev/mv64x60_dev.c                  |   14 +-
 drivers/net/ethernet/marvell/Kconfig               |    1 +
 drivers/net/ethernet/marvell/mv643xx_eth.c         |  187 +-------------------
 drivers/net/ethernet/marvell/mvmdio.c              |  136 +++++++++++---
 7 files changed, 226 insertions(+), 219 deletions(-)

-- 
1.7.10.4

[PATCH 1/5] net: mvmdio: unmap base register address at driver removal

[Florian Fainelli]

Fix the driver remove callback to unmap the base register address and
not leak this mapping after the driver has been removed.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 drivers/net/ethernet/marvell/mvmdio.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 74f1c15..be5c690 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -200,9 +200,12 @@ static int orion_mdio_probe(struct platform_device *pdev)
 static int orion_mdio_remove(struct platform_device *pdev)
 {
 	struct mii_bus *bus = platform_get_drvdata(pdev);
+	struct orion_mdio_dev *dev = bus->priv;
+
 	mdiobus_unregister(bus);
 	kfree(bus->irq);
 	mdiobus_free(bus);
+	iounmap(dev->smireg);
 	return 0;
 }
 
-- 
1.7.10.4

Re: [PATCH 1/5] net: mvmdio: unmap base register address at driver removal

[Thomas Petazzoni]

Dear Florian Fainelli,

On Tue, 29 Jan 2013 16:24:04 +0100, Florian Fainelli wrote:
> Fix the driver remove callback to unmap the base register address and
> not leak this mapping after the driver has been removed.
> 
> Signed-off-by: Florian Fainelli <florian@openwrt.org>

What about using devm_request_and_ioremap() instead, in order to get
automatic unmap on error and in the ->remove() path?

But maybe it won't work because this memory range is claimed both by
the MDIO driver and the Ethernet driver itself. In that case, you could
use devm_ioremap().

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 1/5] net: mvmdio: unmap base register address at driver removal

[Florian Fainelli]

On 01/29/2013 04:32 PM, Thomas Petazzoni wrote:
> Dear Florian Fainelli,
>
> On Tue, 29 Jan 2013 16:24:04 +0100, Florian Fainelli wrote:
>> Fix the driver remove callback to unmap the base register address and
>> not leak this mapping after the driver has been removed.
>>
>> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> What about using devm_request_and_ioremap() instead, in order to get
> automatic unmap on error and in the ->remove() path?

Right now, you are using of_iomap() which eases the task of fetching the 
resource and getting an ioremap cookie, which I why I kept that.

>
> But maybe it won't work because this memory range is claimed both by
> the MDIO driver and the Ethernet driver itself. In that case, you could
> use devm_ioremap().
Then we would loose the facility of of_iomap(), but fair enough, it can 
be inserted as a patch in this serie.

Thanks
--
Florian

[PATCH 2/5] net: mvmdio: rename base register cookie from smireg to regs

[Florian Fainelli]

This patch renames the base register cookie in the mvmdio drive from
"smireg" to "regs" since a subsequent patch is going to use an ioremap()
cookie whose size is larger than a single register of 4 bytes. No
functionnal code change introduced.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 drivers/net/ethernet/marvell/mvmdio.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index be5c690..5ecda58 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -39,7 +39,7 @@
 
 struct orion_mdio_dev {
 	struct mutex lock;
-	void __iomem *smireg;
+	void __iomem *regs;
 };
 
 /* Wait for the SMI unit to be ready for another operation
@@ -52,7 +52,7 @@ static int orion_mdio_wait_ready(struct mii_bus *bus)
 
 	count = 0;
 	while (1) {
-		val = readl(dev->smireg);
+		val = readl(dev->regs);
 		if (!(val & MVMDIO_SMI_BUSY))
 			break;
 
@@ -87,12 +87,12 @@ static int orion_mdio_read(struct mii_bus *bus, int mii_id,
 	writel(((mii_id << MVMDIO_SMI_PHY_ADDR_SHIFT) |
 		(regnum << MVMDIO_SMI_PHY_REG_SHIFT)  |
 		MVMDIO_SMI_READ_OPERATION),
-	       dev->smireg);
+	       dev->regs);
 
 	/* Wait for the value to become available */
 	count = 0;
 	while (1) {
-		val = readl(dev->smireg);
+		val = readl(dev->regs);
 		if (val & MVMDIO_SMI_READ_VALID)
 			break;
 
@@ -129,7 +129,7 @@ static int orion_mdio_write(struct mii_bus *bus, int mii_id,
 		(regnum << MVMDIO_SMI_PHY_REG_SHIFT)  |
 		MVMDIO_SMI_WRITE_OPERATION            |
 		(value << MVMDIO_SMI_DATA_SHIFT)),
-	       dev->smireg);
+	       dev->regs);
 
 	mutex_unlock(&dev->lock);
 
@@ -173,8 +173,8 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		bus->irq[i] = PHY_POLL;
 
 	dev = bus->priv;
-	dev->smireg = of_iomap(pdev->dev.of_node, 0);
-	if (!dev->smireg) {
+	dev->regs = of_iomap(pdev->dev.of_node, 0);
+	if (!dev->regs) {
 		dev_err(&pdev->dev, "No SMI register address given in DT\n");
 		kfree(bus->irq);
 		mdiobus_free(bus);
@@ -186,7 +186,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
 	ret = of_mdiobus_register(bus, np);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
-		iounmap(dev->smireg);
+		iounmap(dev->regs);
 		kfree(bus->irq);
 		mdiobus_free(bus);
 		return ret;
@@ -205,7 +205,7 @@ static int orion_mdio_remove(struct platform_device *pdev)
 	mdiobus_unregister(bus);
 	kfree(bus->irq);
 	mdiobus_free(bus);
-	iounmap(dev->smireg);
+	iounmap(dev->regs);
 	return 0;
 }
 
-- 
1.7.10.4

Re: [PATCH 2/5] net: mvmdio: rename base register cookie from smireg to regs

[Thomas Petazzoni]

Dear Florian Fainelli,

On Tue, 29 Jan 2013 16:24:05 +0100, Florian Fainelli wrote:
> This patch renames the base register cookie in the mvmdio drive from
> "smireg" to "regs" since a subsequent patch is going to use an ioremap()
> cookie whose size is larger than a single register of 4 bytes. No
> functionnal code change introduced.
> 
> Signed-off-by: Florian Fainelli <florian@openwrt.org>

Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 3/5] net: mvmdio: enhance driver to support SMI error/done interrupts

[Florian Fainelli]

This patch enhances the "mvmdio" to support a SMI error/done interrupt
line which can be used along with a wait queue instead of doing
busy-waiting on the registers. This is a feature which is available in
the mv643xx_eth SMI code and thus reduces again the gap between the two.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 .../devicetree/bindings/net/marvell-orion-mdio.txt |    3 +
 drivers/net/ethernet/marvell/mvmdio.c              |   83 +++++++++++++++++---
 2 files changed, 75 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
index 34e7aaf..052b5f2 100644
--- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
+++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
@@ -9,6 +9,9 @@ Required properties:
 - compatible: "marvell,orion-mdio"
 - reg: address and length of the SMI register
 
+Optional properties:
+- interrupts: interrupt line number for the SMI error/done interrupt
+
 The child nodes of the MDIO driver are the individual PHY devices
 connected to this MDIO bus. They must have a "reg" property given the
 PHY address on the MDIO bus.
diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 5ecda58..cada794 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -24,10 +24,14 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/phy.h>
+#include <linux/interrupt.h>
 #include <linux/of_address.h>
 #include <linux/of_mdio.h>
+#include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
+#include <linux/sched.h>
+#include <linux/wait.h>
 
 #define MVMDIO_SMI_DATA_SHIFT              0
 #define MVMDIO_SMI_PHY_ADDR_SHIFT          16
@@ -36,12 +40,28 @@
 #define MVMDIO_SMI_WRITE_OPERATION         0
 #define MVMDIO_SMI_READ_VALID              BIT(27)
 #define MVMDIO_SMI_BUSY                    BIT(28)
+#define MVMDIO_ERR_INT_CAUSE		   0x007C
+#define  MVMDIO_ERR_INT_SMI_DONE	   0x00000010
+#define MVMDIO_ERR_INT_MASK		   0x0080
 
 struct orion_mdio_dev {
 	struct mutex lock;
 	void __iomem *regs;
+	/*
+	 * If we have access to the error interrupt pin (which is
+	 * somewhat misnamed as it not only reflects internal errors
+	 * but also reflects SMI completion), use that to wait for
+	 * SMI access completion instead of polling the SMI busy bit.
+	 */
+	int err_interrupt;
+	wait_queue_head_t smi_busy_wait;
 };
 
+static int orion_mdio_smi_is_done(struct orion_mdio_dev *dev)
+{
+	return !(readl(dev->regs) & MVMDIO_SMI_BUSY);
+}
+
 /* Wait for the SMI unit to be ready for another operation
  */
 static int orion_mdio_wait_ready(struct mii_bus *bus)
@@ -50,19 +70,30 @@ static int orion_mdio_wait_ready(struct mii_bus *bus)
 	int count;
 	u32 val;
 
-	count = 0;
-	while (1) {
-		val = readl(dev->regs);
-		if (!(val & MVMDIO_SMI_BUSY))
-			break;
-
-		if (count > 100) {
-			dev_err(bus->parent, "Timeout: SMI busy for too long\n");
-			return -ETIMEDOUT;
+	if (dev->err_interrupt == NO_IRQ) {
+		count = 0;
+		while (1) {
+			val = readl(dev->regs);
+			if (!(val & MVMDIO_SMI_BUSY))
+				break;
+
+			if (count > 100) {
+				dev_err(bus->parent,
+					"Timeout: SMI busy for too long\n");
+				return -ETIMEDOUT;
+			}
+
+			udelay(10);
+			count++;
 		}
+	}
 
-		udelay(10);
-		count++;
+	if (!orion_mdio_smi_is_done(dev)) {
+		wait_event_timeout(dev->smi_busy_wait,
+				orion_mdio_smi_is_done(dev),
+				msecs_to_jiffies(100));
+		if (!orion_mdio_smi_is_done(dev))
+			return -ETIMEDOUT;
 	}
 
 	return 0;
@@ -141,6 +172,21 @@ static int orion_mdio_reset(struct mii_bus *bus)
 	return 0;
 }
 
+static irqreturn_t orion_mdio_err_irq(int irq, void *dev_id)
+{
+	struct orion_mdio_dev *dev = dev_id;
+
+	if (readl(dev->regs + MVMDIO_ERR_INT_CAUSE) &
+			MVMDIO_ERR_INT_SMI_DONE) {
+		writel(~MVMDIO_ERR_INT_SMI_DONE,
+				dev->regs + MVMDIO_ERR_INT_CAUSE);
+		wake_up(&dev->smi_busy_wait);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
 static int orion_mdio_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -181,6 +227,19 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	dev->err_interrupt = NO_IRQ;
+	init_waitqueue_head(&dev->smi_busy_wait);
+
+	dev->err_interrupt = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	if (dev->err_interrupt != NO_IRQ) {
+		ret = devm_request_irq(&pdev->dev, dev->err_interrupt,
+					orion_mdio_err_irq,
+					IRQF_SHARED, pdev->name, dev);
+		if (!ret)
+			writel(MVMDIO_ERR_INT_SMI_DONE,
+					dev->regs + MVMDIO_ERR_INT_MASK);
+	}
+
 	mutex_init(&dev->lock);
 
 	ret = of_mdiobus_register(bus, np);
@@ -202,6 +261,8 @@ static int orion_mdio_remove(struct platform_device *pdev)
 	struct mii_bus *bus = platform_get_drvdata(pdev);
 	struct orion_mdio_dev *dev = bus->priv;
 
+	writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
+	free_irq(dev->err_interrupt, dev);
 	mdiobus_unregister(bus);
 	kfree(bus->irq);
 	mdiobus_free(bus);
-- 
1.7.10.4

Re: [PATCH 3/5] net: mvmdio: enhance driver to support SMI error/done interrupts

[Thomas Petazzoni]

Dear Florian Fainelli,

On Tue, 29 Jan 2013 16:24:06 +0100, Florian Fainelli wrote:

>  #define MVMDIO_SMI_DATA_SHIFT              0
>  #define MVMDIO_SMI_PHY_ADDR_SHIFT          16
> @@ -36,12 +40,28 @@
>  #define MVMDIO_SMI_WRITE_OPERATION         0
>  #define MVMDIO_SMI_READ_VALID              BIT(27)
>  #define MVMDIO_SMI_BUSY                    BIT(28)
> +#define MVMDIO_ERR_INT_CAUSE		   0x007C
> +#define  MVMDIO_ERR_INT_SMI_DONE	   0x00000010
> +#define MVMDIO_ERR_INT_MASK		   0x0080
>  
>  struct orion_mdio_dev {
>  	struct mutex lock;
>  	void __iomem *regs;
> +	/*
> +	 * If we have access to the error interrupt pin (which is
> +	 * somewhat misnamed as it not only reflects internal errors
> +	 * but also reflects SMI completion), use that to wait for
> +	 * SMI access completion instead of polling the SMI busy bit.
> +	 */
> +	int err_interrupt;
> +	wait_queue_head_t smi_busy_wait;
>  };
>  
> +static int orion_mdio_smi_is_done(struct orion_mdio_dev *dev)
> +{
> +	return !(readl(dev->regs) & MVMDIO_SMI_BUSY);
> +}
> +
>  /* Wait for the SMI unit to be ready for another operation
>   */
>  static int orion_mdio_wait_ready(struct mii_bus *bus)
> @@ -50,19 +70,30 @@ static int orion_mdio_wait_ready(struct mii_bus *bus)
>  	int count;
>  	u32 val;
>  
> -	count = 0;
> -	while (1) {
> -		val = readl(dev->regs);
> -		if (!(val & MVMDIO_SMI_BUSY))
> -			break;
> -
> -		if (count > 100) {
> -			dev_err(bus->parent, "Timeout: SMI busy for too long\n");
> -			return -ETIMEDOUT;
> +	if (dev->err_interrupt == NO_IRQ) {
> +		count = 0;
> +		while (1) {
> +			val = readl(dev->regs);
> +			if (!(val & MVMDIO_SMI_BUSY))
> +				break;

What about using your new orion_mdio_smi_is_done() function here?

> +
> +			if (count > 100) {
> +				dev_err(bus->parent,
> +					"Timeout: SMI busy for too long\n");
> +				return -ETIMEDOUT;
> +			}
> +
> +			udelay(10);
> +			count++;
>  		}
> +	}
>  
> -		udelay(10);
> -		count++;
> +	if (!orion_mdio_smi_is_done(dev)) {

Maybe it should be in an else if block so that the waitqueue case is
only considered if there is an IRQ registered? Of course practically
speaking, it's OK because if there is no IRQ, we'll wait in the polling
loop above, and either exit from the function on timeout, or continue
on success. But it still would make the code a little bit clearer, I'd
say.

>  static int orion_mdio_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> @@ -181,6 +227,19 @@ static int orion_mdio_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> +	dev->err_interrupt = NO_IRQ;

Not needed, you already do dev->err_interrupt = something() below.

> +	init_waitqueue_head(&dev->smi_busy_wait);
> +
> +	dev->err_interrupt = irq_of_parse_and_map(pdev->dev.of_node, 0);
> +	if (dev->err_interrupt != NO_IRQ) {
> +		ret = devm_request_irq(&pdev->dev, dev->err_interrupt,
> +					orion_mdio_err_irq,
> +					IRQF_SHARED, pdev->name, dev);
> +		if (!ret)
> +			writel(MVMDIO_ERR_INT_SMI_DONE,
> +					dev->regs + MVMDIO_ERR_INT_MASK);
> +	}
> +
>  	mutex_init(&dev->lock);
>  
>  	ret = of_mdiobus_register(bus, np);
> @@ -202,6 +261,8 @@ static int orion_mdio_remove(struct platform_device *pdev)
>  	struct mii_bus *bus = platform_get_drvdata(pdev);
>  	struct orion_mdio_dev *dev = bus->priv;
>  
> +	writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
> +	free_irq(dev->err_interrupt, dev);

free_irq() not needed since the IRQ handler is registered with
devm_request_irq().

>  	mdiobus_unregister(bus);
>  	kfree(bus->irq);
>  	mdiobus_free(bus);

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 4/5] net: mvmdio: allow Device Tree and platform device to coexist

[Florian Fainelli]

This patch changes the Marvell MDIO driver to be registered by using
both Device Tree and platform device methods. The driver voluntarily
does not use devm_ioremap() to share the same error path for Device Tree
and non-Device Tree cases.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 drivers/net/ethernet/marvell/mvmdio.c |   46 +++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index cada794..10c593c 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -190,6 +190,7 @@ static irqreturn_t orion_mdio_err_irq(int irq, void *dev_id)
 static int orion_mdio_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
+	struct resource *r = NULL;
 	struct mii_bus *bus;
 	struct orion_mdio_dev *dev;
 	int i, ret;
@@ -219,18 +220,31 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		bus->irq[i] = PHY_POLL;
 
 	dev = bus->priv;
-	dev->regs = of_iomap(pdev->dev.of_node, 0);
-	if (!dev->regs) {
-		dev_err(&pdev->dev, "No SMI register address given in DT\n");
-		kfree(bus->irq);
-		mdiobus_free(bus);
-		return -ENODEV;
-	}
-
 	dev->err_interrupt = NO_IRQ;
 	init_waitqueue_head(&dev->smi_busy_wait);
 
-	dev->err_interrupt = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	if (pdev->dev.of_node) {
+		dev->regs = of_iomap(pdev->dev.of_node, 0);
+		if (!dev->regs) {
+			dev_err(&pdev->dev, "No SMI register address given in DT\n");
+			ret = -ENODEV;
+			goto out_free;
+		}
+
+		dev->err_interrupt = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	} else {
+		r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+		dev->regs = ioremap(r->start, resource_size(r));
+		if (!dev->regs) {
+			dev_err(&pdev->dev, "No SMI register address given\n");
+			ret = -ENODEV;
+			goto out_free;
+		}
+
+		dev->err_interrupt = platform_get_irq(pdev, 0);
+	}
+
 	if (dev->err_interrupt != NO_IRQ) {
 		ret = devm_request_irq(&pdev->dev, dev->err_interrupt,
 					orion_mdio_err_irq,
@@ -242,18 +256,24 @@ static int orion_mdio_probe(struct platform_device *pdev)
 
 	mutex_init(&dev->lock);
 
-	ret = of_mdiobus_register(bus, np);
+	if (np)
+		ret = of_mdiobus_register(bus, np);
+	else
+		ret = mdiobus_register(bus);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
 		iounmap(dev->regs);
-		kfree(bus->irq);
-		mdiobus_free(bus);
-		return ret;
+		goto out_free;
 	}
 
 	platform_set_drvdata(pdev, bus);
 
 	return 0;
+
+out_free:
+	kfree(bus->irq);
+	mdiobus_free(bus);
+	return ret;
 }
 
 static int orion_mdio_remove(struct platform_device *pdev)
-- 
1.7.10.4

Re: [PATCH 4/5] net: mvmdio: allow Device Tree and platform device to coexist

[Thomas Petazzoni]

Dear Florian Fainelli,

On Tue, 29 Jan 2013 16:24:07 +0100, Florian Fainelli wrote:
> This patch changes the Marvell MDIO driver to be registered by using
> both Device Tree and platform device methods. The driver voluntarily
> does not use devm_ioremap() to share the same error path for Device Tree
> and non-Device Tree cases.

Not sure why you think devm_ioremap() can't be used here. Maybe I'm
missing something, but could you explain? If you use devm_ioremap(),
then basically you don't need to do anything in the error path
regarding to the I/O mapping... since it's the whole purpose of the
devm_*() stuff to automagically undo things in the error case, and in
the ->remove() code.

> -	dev->err_interrupt = irq_of_parse_and_map(pdev->dev.of_node, 0);
> +	if (pdev->dev.of_node) {
> +		dev->regs = of_iomap(pdev->dev.of_node, 0);
> +		if (!dev->regs) {
> +			dev_err(&pdev->dev, "No SMI register address given in DT\n");
> +			ret = -ENODEV;
> +			goto out_free;
> +		}
> +
> +		dev->err_interrupt = irq_of_parse_and_map(pdev->dev.of_node, 0);
> +	} else {
> +		r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +		dev->regs = ioremap(r->start, resource_size(r));
> +		if (!dev->regs) {
> +			dev_err(&pdev->dev, "No SMI register address given\n");
> +			ret = -ENODEV;
> +			goto out_free;
> +		}
> +
> +		dev->err_interrupt = platform_get_irq(pdev, 0);
> +	}

I think you can do a devm_ioremap() and a platform_get_irq() in both
cases here, and therefore keep the code common between the DT case and
the !DT case.

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 4/5] net: mvmdio: allow Device Tree and platform device to coexist

[Jason Gunthorpe]

On Tue, Jan 29, 2013 at 04:24:07PM +0100, Florian Fainelli wrote:
  
> -	dev->err_interrupt = irq_of_parse_and_map(pdev->dev.of_node, 0);
> +	if (pdev->dev.of_node) {
> +		dev->regs = of_iomap(pdev->dev.of_node, 0);
> +		if (!dev->regs) {
> +			dev_err(&pdev->dev, "No SMI register address given in DT\n");
> +			ret = -ENODEV;
> +			goto out_free;
> +		}
> +
> +		dev->err_interrupt = irq_of_parse_and_map(pdev->dev.of_node, 0);
> +	} else {
> +		r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +		dev->regs = ioremap(r->start, resource_size(r));
> +		if (!dev->regs) {
> +			dev_err(&pdev->dev, "No SMI register address given\n");
> +			ret = -ENODEV;
> +			goto out_free;
> +		}
> +
> +		dev->err_interrupt = platform_get_irq(pdev, 0);
> +	}

Why do you have these different paths for OF and platform? AFAIK these
days when a OF device is automatically converted into a platform
device all the struct resources are created too, so you can't you just
use platform_get_resource and devm_request_and_ioremap for both flows?

Ditto for the interrupt - platform_get_irq should work in both cases?

Jason

Re: [PATCH 4/5] net: mvmdio: allow Device Tree and platform device to coexist

[Florian Fainelli]

Le mardi 29 janvier 2013 18:59:12, Jason Gunthorpe a écrit :
> On Tue, Jan 29, 2013 at 04:24:07PM +0100, Florian Fainelli wrote:
> > -	dev->err_interrupt = irq_of_parse_and_map(pdev->dev.of_node, 0);
> > +	if (pdev->dev.of_node) {
> > +		dev->regs = of_iomap(pdev->dev.of_node, 0);
> > +		if (!dev->regs) {
> > +			dev_err(&pdev->dev, "No SMI register address given in 
DT\n");
> > +			ret = -ENODEV;
> > +			goto out_free;
> > +		}
> > +
> > +		dev->err_interrupt = irq_of_parse_and_map(pdev->dev.of_node, 0);
> > +	} else {
> > +		r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +
> > +		dev->regs = ioremap(r->start, resource_size(r));
> > +		if (!dev->regs) {
> > +			dev_err(&pdev->dev, "No SMI register address given\n");
> > +			ret = -ENODEV;
> > +			goto out_free;
> > +		}
> > +
> > +		dev->err_interrupt = platform_get_irq(pdev, 0);
> > +	}
> 
> Why do you have these different paths for OF and platform? AFAIK these
> days when a OF device is automatically converted into a platform
> device all the struct resources are created too, so you can't you just
> use platform_get_resource and devm_request_and_ioremap for both flows?
> 
> Ditto for the interrupt - platform_get_irq should work in both cases?

There was no particular reason and I updated the patchset to do that precisely 
in version 2.
-- 
Florian

[PATCH 5/5] mv643xx_eth: convert to use the Marvell Orion MDIO driver

[Florian Fainelli]

This patch converts the Marvell MV643XX ethernet driver to use the
Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
registering the Marvell MV643XX ethernet driver are also updated to
register a Marvell Orion MDIO driver. This driver voluntarily overlaps
with the Marvell Ethernet shared registers because it will use a subset
of this shared register (shared_base + 0x4 - shared_base + 0x84). The
Ethernet driver is also updated to look up for a PHY device using the
Orion MDIO bus driver.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 arch/arm/plat-orion/common.c               |   84 +++++++++++--
 arch/powerpc/platforms/chrp/pegasos_eth.c  |   20 +++
 arch/powerpc/sysdev/mv64x60_dev.c          |   14 ++-
 drivers/net/ethernet/marvell/Kconfig       |    1 +
 drivers/net/ethernet/marvell/mv643xx_eth.c |  187 ++--------------------------
 5 files changed, 113 insertions(+), 193 deletions(-)

diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index 2d4b641..f9bc66e 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -238,6 +238,7 @@ static __init void ge_complete(
 	struct mv643xx_eth_shared_platform_data *orion_ge_shared_data,
 	struct resource *orion_ge_resource, unsigned long irq,
 	struct platform_device *orion_ge_shared,
+	struct platform_device *orion_ge_mvmdio,
 	struct mv643xx_eth_platform_data *eth_data,
 	struct platform_device *orion_ge)
 {
@@ -247,6 +248,7 @@ static __init void ge_complete(
 	orion_ge->dev.platform_data = eth_data;
 
 	platform_device_register(orion_ge_shared);
+	platform_device_register(orion_ge_mvmdio);
 	platform_device_register(orion_ge);
 }
 
@@ -258,8 +260,6 @@ struct mv643xx_eth_shared_platform_data orion_ge00_shared_data;
 static struct resource orion_ge00_shared_resources[] = {
 	{
 		.name	= "ge00 base",
-	}, {
-		.name	= "ge00 err irq",
 	},
 };
 
@@ -271,6 +271,19 @@ static struct platform_device orion_ge00_shared = {
 	},
 };
 
+static struct resource orion_ge00_mvmdio_resources[] = {
+	{
+		.name	= "ge00 mvmdio base",
+	}, {
+		.name	= "ge00 mvmdio err irq",
+	},
+};
+
+static struct platform_device orion_ge00_mvmdio = {
+	.name		= "orion-mdio",
+	.id		= 0,
+};
+
 static struct resource orion_ge00_resources[] = {
 	{
 		.name	= "ge00 irq",
@@ -295,10 +308,13 @@ void __init orion_ge00_init(struct mv643xx_eth_platform_data *eth_data,
 			    unsigned int tx_csum_limit)
 {
 	fill_resources(&orion_ge00_shared, orion_ge00_shared_resources,
-		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+	fill_resources(&orion_ge00_mvmdio, orion_ge00_mvmdio_resources,
+			mapbase + 0x2004, 0x84 - 1, irq_err);
 	orion_ge00_shared_data.tx_csum_limit = tx_csum_limit;
 	ge_complete(&orion_ge00_shared_data,
 		    orion_ge00_resources, irq, &orion_ge00_shared,
+		    &orion_ge00_mvmdio,
 		    eth_data, &orion_ge00);
 }
 
@@ -312,9 +328,7 @@ struct mv643xx_eth_shared_platform_data orion_ge01_shared_data = {
 static struct resource orion_ge01_shared_resources[] = {
 	{
 		.name	= "ge01 base",
-	}, {
-		.name	= "ge01 err irq",
-	},
+	}
 };
 
 static struct platform_device orion_ge01_shared = {
@@ -325,6 +339,19 @@ static struct platform_device orion_ge01_shared = {
 	},
 };
 
+static struct resource orion_ge01_mvmdio_resources[] = {
+	{
+		.name	= "ge01 mdio base",
+	}, {
+		.name	= "ge01 mdio err irq",
+	},
+};
+
+static struct platform_device orion_ge01_mvmdio = {
+	.name		= "orion-mdio",
+	.id		= 1,
+};
+
 static struct resource orion_ge01_resources[] = {
 	{
 		.name	= "ge01 irq",
@@ -349,10 +376,13 @@ void __init orion_ge01_init(struct mv643xx_eth_platform_data *eth_data,
 			    unsigned int tx_csum_limit)
 {
 	fill_resources(&orion_ge01_shared, orion_ge01_shared_resources,
-		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+	fill_resources(&orion_ge01_mvmdio, orion_ge01_mvmdio_resources,
+			mapbase + 0x2004, 0x84 - 1, irq_err);
 	orion_ge01_shared_data.tx_csum_limit = tx_csum_limit;
 	ge_complete(&orion_ge01_shared_data,
 		    orion_ge01_resources, irq, &orion_ge01_shared,
+		    &orion_ge01_mvmdio,
 		    eth_data, &orion_ge01);
 }
 
@@ -366,9 +396,7 @@ struct mv643xx_eth_shared_platform_data orion_ge10_shared_data = {
 static struct resource orion_ge10_shared_resources[] = {
 	{
 		.name	= "ge10 base",
-	}, {
-		.name	= "ge10 err irq",
-	},
+	}
 };
 
 static struct platform_device orion_ge10_shared = {
@@ -379,6 +407,19 @@ static struct platform_device orion_ge10_shared = {
 	},
 };
 
+static struct resource orion_ge10_mvmdio_resources[] = {
+	{
+		.name	= "ge10 mvmdio base",
+	}, {
+		.name	= "ge10 mvmdio err irq",
+	},
+};
+
+static struct platform_device orion_ge10_mvmdio = {
+	.name		= "orion-mdio",
+	.id		= 1,
+};
+
 static struct resource orion_ge10_resources[] = {
 	{
 		.name	= "ge10 irq",
@@ -403,8 +444,11 @@ void __init orion_ge10_init(struct mv643xx_eth_platform_data *eth_data,
 {
 	fill_resources(&orion_ge10_shared, orion_ge10_shared_resources,
 		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+	fill_resources(&orion_ge10_mvmdio, orion_ge10_mvmdio_resources,
+		       mapbase + 0x2004, 0x84 - 1, irq_err);
 	ge_complete(&orion_ge10_shared_data,
 		    orion_ge10_resources, irq, &orion_ge10_shared,
+		    &orion_ge10_mvmdio,
 		    eth_data, &orion_ge10);
 }
 
@@ -418,8 +462,6 @@ struct mv643xx_eth_shared_platform_data orion_ge11_shared_data = {
 static struct resource orion_ge11_shared_resources[] = {
 	{
 		.name	= "ge11 base",
-	}, {
-		.name	= "ge11 err irq",
 	},
 };
 
@@ -431,6 +473,19 @@ static struct platform_device orion_ge11_shared = {
 	},
 };
 
+static struct resource orion_ge11_mvmdio_resources[] = {
+	{
+		.name	= "ge11 mvmdio base",
+	}, {
+		.name	= "ge11 mvmdio err irq",
+	},
+};
+
+static struct platform_device orion_ge11_mvmdio = {
+	.name		= "orion-mdio",
+	.id		= 1,
+};
+
 static struct resource orion_ge11_resources[] = {
 	{
 		.name	= "ge11 irq",
@@ -454,9 +509,12 @@ void __init orion_ge11_init(struct mv643xx_eth_platform_data *eth_data,
 			    unsigned long irq_err)
 {
 	fill_resources(&orion_ge11_shared, orion_ge11_shared_resources,
-		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+	fill_resources(&orion_ge11_mvmdio, orion_ge11_mvmdio_resources,
+		       mapbase + 0x2004, 0x84 - 1, irq_err);
 	ge_complete(&orion_ge11_shared_data,
 		    orion_ge11_resources, irq, &orion_ge11_shared,
+		    &orion_ge11_mvmdio,
 		    eth_data, &orion_ge11);
 }
 
diff --git a/arch/powerpc/platforms/chrp/pegasos_eth.c b/arch/powerpc/platforms/chrp/pegasos_eth.c
index 039fc8e..a671508 100644
--- a/arch/powerpc/platforms/chrp/pegasos_eth.c
+++ b/arch/powerpc/platforms/chrp/pegasos_eth.c
@@ -47,6 +47,25 @@ static struct platform_device mv643xx_eth_shared_device = {
 	.resource	= mv643xx_eth_shared_resources,
 };
 
+/*
+ * The orion mdio driver only covers shared + 0x4 up to shared + 0x84 - 1
+ */
+static struct resource mv643xx_eth_mvmdio_resources[] = {
+	[0] = {
+		.name	= "ethernet mdio base",
+		.start	= 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x4,
+		.end	= 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x83,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct platform_device mv643xx_eth_mvmdio_device = {
+	.name		= "orion-mdio",
+	.id		= 0,
+	.num_resources	= ARRAY_SIZE(mv643xx_eth_mvmdio_resources),
+	.resource	= mv643xx_eth_shared_resources,
+};
+
 static struct resource mv643xx_eth_port1_resources[] = {
 	[0] = {
 		.name	= "eth port1 irq",
@@ -82,6 +101,7 @@ static struct platform_device eth_port1_device = {
 
 static struct platform_device *mv643xx_eth_pd_devs[] __initdata = {
 	&mv643xx_eth_shared_device,
+	&mv643xx_eth_mvmdio_device,
 	&eth_port1_device,
 };
 
diff --git a/arch/powerpc/sysdev/mv64x60_dev.c b/arch/powerpc/sysdev/mv64x60_dev.c
index 0f6af41..630cea3 100644
--- a/arch/powerpc/sysdev/mv64x60_dev.c
+++ b/arch/powerpc/sysdev/mv64x60_dev.c
@@ -214,15 +214,25 @@ static struct platform_device * __init mv64x60_eth_register_shared_pdev(
 						struct device_node *np, int id)
 {
 	struct platform_device *pdev;
-	struct resource r[1];
+	struct resource r[2];
 	int err;
 
 	err = of_address_to_resource(np, 0, &r[0]);
 	if (err)
 		return ERR_PTR(err);
 
+	/* register an orion mdio bus driver */
+	r[1].start = r[0].start + 0x4;
+	r[1].end = r[0].start + 0x84 - 1;
+	r[1].flags = IORESOURCE_MEM;
+
+	pdev = platform_device_register_simple("orion-mdio", id, &r[1], 1);
+	if (!pdev)
+		return pdev;
+
 	pdev = platform_device_register_simple(MV643XX_ETH_SHARED_NAME, id,
-					       r, 1);
+					       &r[0], 1);
+
 	return pdev;
 }
 
diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index edfba93..df06061 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -23,6 +23,7 @@ config MV643XX_ETH
 	depends on (MV64X60 || PPC32 || PLAT_ORION) && INET
 	select INET_LRO
 	select PHYLIB
+	select MVMDIO
 	---help---
 	  This driver supports the gigabit ethernet MACs in the
 	  Marvell Discovery PPC/MIPS chipset family (MV643XX) and
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index c27b23d8..8ef186f 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -67,14 +67,6 @@ static char mv643xx_eth_driver_version[] = "1.4";
  * Registers shared between all ports.
  */
 #define PHY_ADDR			0x0000
-#define SMI_REG				0x0004
-#define  SMI_BUSY			0x10000000
-#define  SMI_READ_VALID			0x08000000
-#define  SMI_OPCODE_READ		0x04000000
-#define  SMI_OPCODE_WRITE		0x00000000
-#define ERR_INT_CAUSE			0x0080
-#define  ERR_INT_SMI_DONE		0x00000010
-#define ERR_INT_MASK			0x0084
 #define WINDOW_BASE(w)			(0x0200 + ((w) << 3))
 #define WINDOW_SIZE(w)			(0x0204 + ((w) << 3))
 #define WINDOW_REMAP_HIGH(w)		(0x0280 + ((w) << 2))
@@ -264,25 +256,6 @@ struct mv643xx_eth_shared_private {
 	void __iomem *base;
 
 	/*
-	 * Points at the right SMI instance to use.
-	 */
-	struct mv643xx_eth_shared_private *smi;
-
-	/*
-	 * Provides access to local SMI interface.
-	 */
-	struct mii_bus *smi_bus;
-
-	/*
-	 * If we have access to the error interrupt pin (which is
-	 * somewhat misnamed as it not only reflects internal errors
-	 * but also reflects SMI completion), use that to wait for
-	 * SMI access completion instead of polling the SMI busy bit.
-	 */
-	int err_interrupt;
-	wait_queue_head_t smi_busy_wait;
-
-	/*
 	 * Per-port MBUS window access register value.
 	 */
 	u32 win_protect;
@@ -1080,98 +1053,6 @@ static void txq_set_fixed_prio_mode(struct tx_queue *txq)
 }
 
 
-/* mii management interface *************************************************/
-static irqreturn_t mv643xx_eth_err_irq(int irq, void *dev_id)
-{
-	struct mv643xx_eth_shared_private *msp = dev_id;
-
-	if (readl(msp->base + ERR_INT_CAUSE) & ERR_INT_SMI_DONE) {
-		writel(~ERR_INT_SMI_DONE, msp->base + ERR_INT_CAUSE);
-		wake_up(&msp->smi_busy_wait);
-		return IRQ_HANDLED;
-	}
-
-	return IRQ_NONE;
-}
-
-static int smi_is_done(struct mv643xx_eth_shared_private *msp)
-{
-	return !(readl(msp->base + SMI_REG) & SMI_BUSY);
-}
-
-static int smi_wait_ready(struct mv643xx_eth_shared_private *msp)
-{
-	if (msp->err_interrupt == NO_IRQ) {
-		int i;
-
-		for (i = 0; !smi_is_done(msp); i++) {
-			if (i == 10)
-				return -ETIMEDOUT;
-			msleep(10);
-		}
-
-		return 0;
-	}
-
-	if (!smi_is_done(msp)) {
-		wait_event_timeout(msp->smi_busy_wait, smi_is_done(msp),
-				   msecs_to_jiffies(100));
-		if (!smi_is_done(msp))
-			return -ETIMEDOUT;
-	}
-
-	return 0;
-}
-
-static int smi_bus_read(struct mii_bus *bus, int addr, int reg)
-{
-	struct mv643xx_eth_shared_private *msp = bus->priv;
-	void __iomem *smi_reg = msp->base + SMI_REG;
-	int ret;
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	writel(SMI_OPCODE_READ | (reg << 21) | (addr << 16), smi_reg);
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	ret = readl(smi_reg);
-	if (!(ret & SMI_READ_VALID)) {
-		pr_warn("SMI bus read not valid\n");
-		return -ENODEV;
-	}
-
-	return ret & 0xffff;
-}
-
-static int smi_bus_write(struct mii_bus *bus, int addr, int reg, u16 val)
-{
-	struct mv643xx_eth_shared_private *msp = bus->priv;
-	void __iomem *smi_reg = msp->base + SMI_REG;
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	writel(SMI_OPCODE_WRITE | (reg << 21) |
-		(addr << 16) | (val & 0xffff), smi_reg);
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	return 0;
-}
-
-
 /* statistics ***************************************************************/
 static struct net_device_stats *mv643xx_eth_get_stats(struct net_device *dev)
 {
@@ -2611,47 +2492,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 		goto out_free;
 
 	/*
-	 * Set up and register SMI bus.
-	 */
-	if (pd == NULL || pd->shared_smi == NULL) {
-		msp->smi_bus = mdiobus_alloc();
-		if (msp->smi_bus == NULL)
-			goto out_unmap;
-
-		msp->smi_bus->priv = msp;
-		msp->smi_bus->name = "mv643xx_eth smi";
-		msp->smi_bus->read = smi_bus_read;
-		msp->smi_bus->write = smi_bus_write,
-		snprintf(msp->smi_bus->id, MII_BUS_ID_SIZE, "%s-%d",
-			pdev->name, pdev->id);
-		msp->smi_bus->parent = &pdev->dev;
-		msp->smi_bus->phy_mask = 0xffffffff;
-		if (mdiobus_register(msp->smi_bus) < 0)
-			goto out_free_mii_bus;
-		msp->smi = msp;
-	} else {
-		msp->smi = platform_get_drvdata(pd->shared_smi);
-	}
-
-	msp->err_interrupt = NO_IRQ;
-	init_waitqueue_head(&msp->smi_busy_wait);
-
-	/*
-	 * Check whether the error interrupt is hooked up.
-	 */
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (res != NULL) {
-		int err;
-
-		err = request_irq(res->start, mv643xx_eth_err_irq,
-				  IRQF_SHARED, "mv643xx_eth", msp);
-		if (!err) {
-			writel(ERR_INT_SMI_DONE, msp->base + ERR_INT_MASK);
-			msp->err_interrupt = res->start;
-		}
-	}
-
-	/*
 	 * (Re-)program MBUS remapping windows if we are asked to.
 	 */
 	dram = mv_mbus_dram_info();
@@ -2666,10 +2506,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 
 	return 0;
 
-out_free_mii_bus:
-	mdiobus_free(msp->smi_bus);
-out_unmap:
-	iounmap(msp->base);
 out_free:
 	kfree(msp);
 out:
@@ -2679,14 +2515,7 @@ out:
 static int mv643xx_eth_shared_remove(struct platform_device *pdev)
 {
 	struct mv643xx_eth_shared_private *msp = platform_get_drvdata(pdev);
-	struct mv643xx_eth_shared_platform_data *pd = pdev->dev.platform_data;
 
-	if (pd == NULL || pd->shared_smi == NULL) {
-		mdiobus_unregister(msp->smi_bus);
-		mdiobus_free(msp->smi_bus);
-	}
-	if (msp->err_interrupt != NO_IRQ)
-		free_irq(msp->err_interrupt, msp);
 	iounmap(msp->base);
 	kfree(msp);
 
@@ -2752,11 +2581,11 @@ static void set_params(struct mv643xx_eth_private *mp,
 static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
 				   int phy_addr)
 {
-	struct mii_bus *bus = mp->shared->smi->smi_bus;
 	struct phy_device *phydev;
 	int start;
 	int num;
 	int i;
+	char phy_id[MII_BUS_ID_SIZE + 3];
 
 	if (phy_addr == MV643XX_ETH_PHY_ADDR_DEFAULT) {
 		start = phy_addr_get(mp) & 0x1f;
@@ -2766,17 +2595,19 @@ static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
 		num = 1;
 	}
 
+	/* Attempt to connect to the PHY using orion-mdio */
 	phydev = NULL;
 	for (i = 0; i < num; i++) {
 		int addr = (start + i) & 0x1f;
 
-		if (bus->phy_map[addr] == NULL)
-			mdiobus_scan(bus, addr);
+		snprintf(phy_id, sizeof(phy_id), PHY_ID_FMT,
+				"orion-mdio-mii", addr);
 
-		if (phydev == NULL) {
-			phydev = bus->phy_map[addr];
-			if (phydev != NULL)
-				phy_addr_set(mp, addr);
+		phydev = phy_connect(mp->dev, phy_id, NULL,
+				PHY_INTERFACE_MODE_GMII);
+		if (!IS_ERR(phydev)) {
+			phy_addr_set(mp, addr);
+			break;
 		}
 	}
 
-- 
1.7.10.4

Re: [PATCH 5/5] mv643xx_eth: convert to use the Marvell Orion MDIO driver

[Thomas Petazzoni]

Dear Florian Fainelli,

On Tue, 29 Jan 2013 16:24:08 +0100, Florian Fainelli wrote:
> This patch converts the Marvell MV643XX ethernet driver to use the
> Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
> registering the Marvell MV643XX ethernet driver are also updated to
> register a Marvell Orion MDIO driver. This driver voluntarily overlaps
> with the Marvell Ethernet shared registers because it will use a subset
> of this shared register (shared_base + 0x4 - shared_base + 0x84). The
> Ethernet driver is also updated to look up for a PHY device using the
> Orion MDIO bus driver.
> 
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> ---
>  arch/arm/plat-orion/common.c               |   84 +++++++++++--

In this file, there was one "MV643XX_ETH_SHARED_NAME" platform_device
registered for each network interface. Why? If the driver is shared,
isn't the whole idea to register it only once?

In any case, one of the idea of separating the mvmdio driver from the
mvneta driver in the first place is that there should be only one
instance of the mvmdio device, even if there are multiple network
interfaces. The reason is that from a HW point of the view, the MDIO
unit is shared between the network interfaces. If you look at
armada-370-xp.dtsi, there is only one mvmdio device registered, and two
network interfaces (using the mvneta driver) that are registered (and
actually up to four network interfaces can exist, they are added by
some other .dtsi files depending on the specific SoC).

So I don't think there should be one instance of the mvmdio per network
interface.

Also, I am wondering what's left in this MV643XX_ETH_SHARED_NAME driver
once the MDIO stuff has been pulled out in a separate driver? I think
the whole point of this work should be to get rid of this
MV643XX_ETH_SHARED_NAME driver, no?

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 5/5] mv643xx_eth: convert to use the Marvell Orion MDIO driver

[Florian Fainelli]

On 01/29/2013 05:01 PM, Thomas Petazzoni wrote:
> Dear Florian Fainelli,
>
> On Tue, 29 Jan 2013 16:24:08 +0100, Florian Fainelli wrote:
>> This patch converts the Marvell MV643XX ethernet driver to use the
>> Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
>> registering the Marvell MV643XX ethernet driver are also updated to
>> register a Marvell Orion MDIO driver. This driver voluntarily overlaps
>> with the Marvell Ethernet shared registers because it will use a subset
>> of this shared register (shared_base + 0x4 - shared_base + 0x84). The
>> Ethernet driver is also updated to look up for a PHY device using the
>> Orion MDIO bus driver.
>>
>> Signed-off-by: Florian Fainelli <florian@openwrt.org>
>> ---
>>   arch/arm/plat-orion/common.c               |   84 +++++++++++--
> In this file, there was one "MV643XX_ETH_SHARED_NAME" platform_device
> registered for each network interface. Why? If the driver is shared,
> isn't the whole idea to register it only once?
It looks like I introduced two redundant mvmdio instances as ge01 refers 
to the ge00 smi bus (the same applies to ge11 and ge10). Thanks for 
spotting this.

>
> In any case, one of the idea of separating the mvmdio driver from the
> mvneta driver in the first place is that there should be only one
> instance of the mvmdio device, even if there are multiple network
> interfaces. The reason is that from a HW point of the view, the MDIO
> unit is shared between the network interfaces. If you look at
> armada-370-xp.dtsi, there is only one mvmdio device registered, and two
> network interfaces (using the mvneta driver) that are registered (and
> actually up to four network interfaces can exist, they are added by
> some other .dtsi files depending on the specific SoC).
>
> So I don't think there should be one instance of the mvmdio per network
> interface.
>
> Also, I am wondering what's left in this MV643XX_ETH_SHARED_NAME driver
> once the MDIO stuff has been pulled out in a separate driver? I think
> the whole point of this work should be to get rid of this
> MV643XX_ETH_SHARED_NAME driver, no?

If you take a closer look at mv643xx_eth you will see that the "shared" 
driver still handles the mconf bus window configuration, which is not 
abstracted yet. Besides that, I would rather do it step by step.
--
Florian

Re: [PATCH 5/5] mv643xx_eth: convert to use the Marvell Orion MDIO driver

[Thomas Petazzoni]

Dear Florian Fainelli,

On Tue, 29 Jan 2013 17:27:56 +0100, Florian Fainelli wrote:

> It looks like I introduced two redundant mvmdio instances as ge01
> refers to the ge00 smi bus (the same applies to ge11 and ge10).
> Thanks for spotting this.

Ok, good.

> If you take a closer look at mv643xx_eth you will see that the
> "shared" driver still handles the mconf bus window configuration,
> which is not abstracted yet.

Indeed, I've seen that. But I don't understand why it's done in the
mv643xx_eth_shared_probe(). The mbus window configuration registers are
per-network interface, so this call to mv643xx_eth_conf_mbus_windows()
could presumably be done in mv643xx_eth_probe().

At least in mvneta, we have the same registers, and we do their
initialization in the driver normal (and only) ->probe() routine.

> Besides that, I would rather do it step by step.

Yes, agreed. But I think it would be good to have followed patches that
progressively get rid of the shared driver thing, as it will help in
bringing a proper DT binding in the mv643xx_eth driver. But it
certainly doesn't need to be part of this specific patch.

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 5/5] mv643xx_eth: convert to use the Marvell Orion MDIO driver

[Jason Gunthorpe]

On Tue, Jan 29, 2013 at 04:24:08PM +0100, Florian Fainelli wrote:
> This patch converts the Marvell MV643XX ethernet driver to use the
> Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
> registering the Marvell MV643XX ethernet driver are also updated to
> register a Marvell Orion MDIO driver. This driver voluntarily overlaps
> with the Marvell Ethernet shared registers because it will use a subset
> of this shared register (shared_base + 0x4 - shared_base + 0x84). The
> Ethernet driver is also updated to look up for a PHY device using the
> Orion MDIO bus driver.

Can you finish off this job by making the mv643xx_eth driver accept
the standard phy-handle OF property instead of using a phy address?

Ie the end result should be something like:

                smi0: mdio@72000 {
                        device_type = "mdio";
                        compatible = "marvell,orion-mdio";
                        reg = <0x72004 0x4>;

                        #address-cells = <1>;
                        #size-cells = <0>;
                        PHY1: ethernet-phy@1 {
                                reg = <1>;
                                device_type = "ethernet-phy";
                                phy-id = <0x01410e90>;
                        };
                };

                egiga0 {
                        device_type = "network";
                        compatible = "marvell,mv643xx-eth";
                        reg = <0x72000 0x4000>;
                        port_number = <0>;
                        phy-handle = <&PHY1>;
                        interrupts = <11>;
                        local-mac-address = [000000000002];  /* Filled by boot loader */
                };

Regards,
Jason

Re: [PATCH 5/5] mv643xx_eth: convert to use the Marvell Orion MDIO driver

[Florian Fainelli]

Le mardi 29 janvier 2013 19:13:06, Jason Gunthorpe a écrit :
> On Tue, Jan 29, 2013 at 04:24:08PM +0100, Florian Fainelli wrote:
> > This patch converts the Marvell MV643XX ethernet driver to use the
> > Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
> > registering the Marvell MV643XX ethernet driver are also updated to
> > register a Marvell Orion MDIO driver. This driver voluntarily overlaps
> > with the Marvell Ethernet shared registers because it will use a subset
> > of this shared register (shared_base + 0x4 - shared_base + 0x84). The
> > Ethernet driver is also updated to look up for a PHY device using the
> > Orion MDIO bus driver.
> 
> Can you finish off this job by making the mv643xx_eth driver accept
> the standard phy-handle OF property instead of using a phy address?

I can certainly do that, at the same time we need to continue supporting the 
"old" platform device style registration without breaking them (PowerPC in 
particular, and the hopefully yet to be converted orion5x). So the phy_scan() 
as I modified it will probably still be there.

> 
> Ie the end result should be something like:
> 
>                 smi0: mdio@72000 {
>                         device_type = "mdio";
>                         compatible = "marvell,orion-mdio";
>                         reg = <0x72004 0x4>;
> 
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         PHY1: ethernet-phy@1 {
>                                 reg = <1>;
>                                 device_type = "ethernet-phy";
>                                 phy-id = <0x01410e90>;
>                         };
>                 };
> 
>                 egiga0 {
>                         device_type = "network";
>                         compatible = "marvell,mv643xx-eth";
>                         reg = <0x72000 0x4000>;
>                         port_number = <0>;
>                         phy-handle = <&PHY1>;
>                         interrupts = <11>;
>                         local-mac-address = [000000000002];  /* Filled by
> boot loader */ };
> 
> Regards,
> Jason

-- 
Florian

Re: [PATCH 0/5] mv643xx_eth: use mvmdio MDIO bus driver

[Jason Cooper]

Florian,

Any word on version 2 of this series?  I'd like to base the conversion
of kirkwood to DT based ethernet init on it.

thx,

Jason.

On Tue, Jan 29, 2013 at 04:24:03PM +0100, Florian Fainelli wrote:
> Hi all,
> 
> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
> instead of rolling its own implementation. As a result, all users of this
> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
> The mvmdio driver is also updated to support an interrupt line which reports
> SMI error/completion, and to allow traditionnal platform device registration
> instead of just device tree.
> 
> David, I think it makes sense for you to merge all of this, since we do
> not want the architecture files to be desynchronized from the mv643xx_eth to
> avoid runtime breakage. The potential for merge conflicts should be very small.
> 
> You can probably safely merge patches 1 to 4 if Thomas agrees, and we will
> see what kind of feeback I get on patch 5.
> 
> Florian Fainelli (5):
>   net: mvmdio: unmap base register address at driver removal
>   net: mvmdio: rename base register cookie from smireg to regs
>   net: mvmdio: enhance driver to support SMI error/done interrupts
>   net: mvmdio: allow Device Tree and platform device to coexist
>   mv643xx_eth: convert to use the Marvell Orion MDIO driver
> 
>  .../devicetree/bindings/net/marvell-orion-mdio.txt |    3 +
>  arch/arm/plat-orion/common.c                       |   84 +++++++--
>  arch/powerpc/platforms/chrp/pegasos_eth.c          |   20 +++
>  arch/powerpc/sysdev/mv64x60_dev.c                  |   14 +-
>  drivers/net/ethernet/marvell/Kconfig               |    1 +
>  drivers/net/ethernet/marvell/mv643xx_eth.c         |  187 +-------------------
>  drivers/net/ethernet/marvell/mvmdio.c              |  136 +++++++++++---
>  7 files changed, 226 insertions(+), 219 deletions(-)
> 
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/5] mv643xx_eth: use mvmdio MDIO bus driver

[Florian Fainelli]

Hello Jason,

Le 03/14/13 18:25, Jason Cooper a écrit :
> Florian,
>
> Any word on version 2 of this series?  I'd like to base the conversion
> of kirkwood to DT based ethernet init on it.

Just sent them out for review, thanks for reminder, they were sitting in 
my tree for a couple days already.
--
Florian

Re: [PATCH 0/5] mv643xx_eth: use mvmdio MDIO bus driver

[Jason Cooper]

On Thu, Mar 14, 2013 at 07:09:18PM +0100, Florian Fainelli wrote:
> Hello Jason,
> 
> Le 03/14/13 18:25, Jason Cooper a écrit :
> >Florian,
> >
> >Any word on version 2 of this series?  I'd like to base the conversion
> >of kirkwood to DT based ethernet init on it.
> 
> Just sent them out for review, thanks for reminder, they were
> sitting in my tree for a couple days already.

Great!  I'll give this a spin.

thx,

Jason.

[PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver

[Florian Fainelli]

Hi all,

This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
instead of rolling its own implementation. As a result, all users of this
mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
The mvmdio driver is also updated to support an interrupt line which reports
SMI error/completion, and to allow traditionnal platform device registration
instead of just device tree.

David, I think it makes sense for you to merge all of this, since we do
not want the architecture files to be desynchronized from the mv643xx_eth to
avoid runtime breakage. The potential for merge conflicts should be very small.

Florian Fainelli (4):
  net: mvmdio: allow platform device style registration
  net: mvmdio: rename base register cookie from smireg to regs
  net: mvmdio: enhance driver to support SMI error/done interrupts
  mv643xx_eth: convert to use the Marvell Orion MDIO driver

 .../devicetree/bindings/net/marvell-orion-mdio.txt |    3 +
 arch/arm/plat-orion/common.c                       |   97 +++++++---
 arch/powerpc/platforms/chrp/pegasos_eth.c          |   20 +++
 arch/powerpc/sysdev/mv64x60_dev.c                  |   14 +-
 drivers/net/ethernet/marvell/Kconfig               |    1 +
 drivers/net/ethernet/marvell/mv643xx_eth.c         |  186 +-------------------
 drivers/net/ethernet/marvell/mvmdio.c              |  111 +++++++++---
 include/linux/mv643xx_eth.h                        |    1 -
 8 files changed, 207 insertions(+), 226 deletions(-)

-- 
1.7.10.4

[PATCH] mv643xx_eth: convert to use the Marvell Orion MDIO driver

[Florian Fainelli]

This patch converts the Marvell MV643XX ethernet driver to use the
Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
registering the Marvell MV643XX ethernet driver are also updated to
register a Marvell Orion MDIO driver. This driver voluntarily overlaps
with the Marvell Ethernet shared registers because it will use a subset
of this shared register (shared_base + 0x4 - shared_base + 0x84). The
Ethernet driver is also updated to look up for a PHY device using the
Orion MDIO bus driver.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 arch/arm/plat-orion/common.c               |   97 +++++++++++----
 arch/powerpc/platforms/chrp/pegasos_eth.c  |   20 +++
 arch/powerpc/sysdev/mv64x60_dev.c          |   14 ++-
 drivers/net/ethernet/marvell/Kconfig       |    1 +
 drivers/net/ethernet/marvell/mv643xx_eth.c |  186 ++--------------------------
 include/linux/mv643xx_eth.h                |    1 -
 6 files changed, 117 insertions(+), 202 deletions(-)

diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index 2d4b641..dc4345e 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -238,6 +238,7 @@ static __init void ge_complete(
 	struct mv643xx_eth_shared_platform_data *orion_ge_shared_data,
 	struct resource *orion_ge_resource, unsigned long irq,
 	struct platform_device *orion_ge_shared,
+	struct platform_device *orion_ge_mvmdio,
 	struct mv643xx_eth_platform_data *eth_data,
 	struct platform_device *orion_ge)
 {
@@ -247,6 +248,8 @@ static __init void ge_complete(
 	orion_ge->dev.platform_data = eth_data;
 
 	platform_device_register(orion_ge_shared);
+	if (orion_ge_mvmdio)
+		platform_device_register(orion_ge_mvmdio);
 	platform_device_register(orion_ge);
 }
 
@@ -258,8 +261,6 @@ struct mv643xx_eth_shared_platform_data orion_ge00_shared_data;
 static struct resource orion_ge00_shared_resources[] = {
 	{
 		.name	= "ge00 base",
-	}, {
-		.name	= "ge00 err irq",
 	},
 };
 
@@ -271,6 +272,19 @@ static struct platform_device orion_ge00_shared = {
 	},
 };
 
+static struct resource orion_ge00_mvmdio_resources[] = {
+	{
+		.name	= "ge00 mvmdio base",
+	}, {
+		.name	= "ge00 mvmdio irr irq",
+	},
+};
+
+static struct platform_device orion_ge00_mvmdio = {
+	.name		= "orion-mdio",
+	.id		= 0,
+};
+
 static struct resource orion_ge00_resources[] = {
 	{
 		.name	= "ge00 irq",
@@ -295,26 +309,25 @@ void __init orion_ge00_init(struct mv643xx_eth_platform_data *eth_data,
 			    unsigned int tx_csum_limit)
 {
 	fill_resources(&orion_ge00_shared, orion_ge00_shared_resources,
-		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+	fill_resources(&orion_ge00_mvmdio, orion_ge00_mvmdio_resources,
+			mapbase + 0x2004, 0x84 - 1, irq_err);
 	orion_ge00_shared_data.tx_csum_limit = tx_csum_limit;
 	ge_complete(&orion_ge00_shared_data,
 		    orion_ge00_resources, irq, &orion_ge00_shared,
+		    &orion_ge00_mvmdio,
 		    eth_data, &orion_ge00);
 }
 
 /*****************************************************************************
  * GE01
  ****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge01_shared_data = {
-	.shared_smi	= &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge01_shared_data;
 
 static struct resource orion_ge01_shared_resources[] = {
 	{
 		.name	= "ge01 base",
-	}, {
-		.name	= "ge01 err irq",
-	},
+	}
 };
 
 static struct platform_device orion_ge01_shared = {
@@ -325,6 +338,19 @@ static struct platform_device orion_ge01_shared = {
 	},
 };
 
+static struct resource orion_ge01_mvmdio_resources[] = {
+	{
+		.name	= "ge01 mdio base",
+	}, {
+		.name	= "ge01 mdio err irq",
+	},
+};
+
+static struct platform_device orion_ge01_mvmdio = {
+	.name		= "orion-mdio",
+	.id		= 1,
+};
+
 static struct resource orion_ge01_resources[] = {
 	{
 		.name	= "ge01 irq",
@@ -349,26 +375,25 @@ void __init orion_ge01_init(struct mv643xx_eth_platform_data *eth_data,
 			    unsigned int tx_csum_limit)
 {
 	fill_resources(&orion_ge01_shared, orion_ge01_shared_resources,
-		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+	fill_resources(&orion_ge01_mvmdio, orion_ge01_mvmdio_resources,
+			mapbase + 0x2004, 0x84 - 1, irq_err);
 	orion_ge01_shared_data.tx_csum_limit = tx_csum_limit;
 	ge_complete(&orion_ge01_shared_data,
 		    orion_ge01_resources, irq, &orion_ge01_shared,
+		    NULL,
 		    eth_data, &orion_ge01);
 }
 
 /*****************************************************************************
  * GE10
  ****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge10_shared_data = {
-	.shared_smi	= &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge10_shared_data;
 
 static struct resource orion_ge10_shared_resources[] = {
 	{
 		.name	= "ge10 base",
-	}, {
-		.name	= "ge10 err irq",
-	},
+	}
 };
 
 static struct platform_device orion_ge10_shared = {
@@ -379,6 +404,19 @@ static struct platform_device orion_ge10_shared = {
 	},
 };
 
+static struct resource orion_ge10_mvmdio_resources[] = {
+	{
+		.name	= "ge10 mvmdio base",
+	}, {
+		.name	= "ge10 mvmdio err irq",
+	},
+};
+
+static struct platform_device orion_ge10_mvmdio = {
+	.name		= "orion-mdio",
+	.id		= 1,
+};
+
 static struct resource orion_ge10_resources[] = {
 	{
 		.name	= "ge10 irq",
@@ -403,23 +441,22 @@ void __init orion_ge10_init(struct mv643xx_eth_platform_data *eth_data,
 {
 	fill_resources(&orion_ge10_shared, orion_ge10_shared_resources,
 		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+	fill_resources(&orion_ge10_mvmdio, orion_ge10_mvmdio_resources,
+		       mapbase + 0x2004, 0x84 - 1, irq_err);
 	ge_complete(&orion_ge10_shared_data,
 		    orion_ge10_resources, irq, &orion_ge10_shared,
+		    &orion_ge10_mvmdio,
 		    eth_data, &orion_ge10);
 }
 
 /*****************************************************************************
  * GE11
  ****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge11_shared_data = {
-	.shared_smi	= &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge11_shared_data;
 
 static struct resource orion_ge11_shared_resources[] = {
 	{
 		.name	= "ge11 base",
-	}, {
-		.name	= "ge11 err irq",
 	},
 };
 
@@ -431,6 +468,19 @@ static struct platform_device orion_ge11_shared = {
 	},
 };
 
+static struct resource orion_ge11_mvmdio_resources[] = {
+	{
+		.name	= "ge11 mvmdio base",
+	}, {
+		.name	= "ge11 mvmdio err irq",
+	},
+};
+
+static struct platform_device orion_ge11_mvmdio = {
+	.name		= "orion-mdio",
+	.id		= 1,
+};
+
 static struct resource orion_ge11_resources[] = {
 	{
 		.name	= "ge11 irq",
@@ -454,9 +504,12 @@ void __init orion_ge11_init(struct mv643xx_eth_platform_data *eth_data,
 			    unsigned long irq_err)
 {
 	fill_resources(&orion_ge11_shared, orion_ge11_shared_resources,
-		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+	fill_resources(&orion_ge11_mvmdio, orion_ge11_mvmdio_resources,
+		       mapbase + 0x2004, 0x84 - 1, irq_err);
 	ge_complete(&orion_ge11_shared_data,
 		    orion_ge11_resources, irq, &orion_ge11_shared,
+		    NULL,
 		    eth_data, &orion_ge11);
 }
 
diff --git a/arch/powerpc/platforms/chrp/pegasos_eth.c b/arch/powerpc/platforms/chrp/pegasos_eth.c
index 039fc8e..a671508 100644
--- a/arch/powerpc/platforms/chrp/pegasos_eth.c
+++ b/arch/powerpc/platforms/chrp/pegasos_eth.c
@@ -47,6 +47,25 @@ static struct platform_device mv643xx_eth_shared_device = {
 	.resource	= mv643xx_eth_shared_resources,
 };
 
+/*
+ * The orion mdio driver only covers shared + 0x4 up to shared + 0x84 - 1
+ */
+static struct resource mv643xx_eth_mvmdio_resources[] = {
+	[0] = {
+		.name	= "ethernet mdio base",
+		.start	= 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x4,
+		.end	= 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x83,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct platform_device mv643xx_eth_mvmdio_device = {
+	.name		= "orion-mdio",
+	.id		= 0,
+	.num_resources	= ARRAY_SIZE(mv643xx_eth_mvmdio_resources),
+	.resource	= mv643xx_eth_shared_resources,
+};
+
 static struct resource mv643xx_eth_port1_resources[] = {
 	[0] = {
 		.name	= "eth port1 irq",
@@ -82,6 +101,7 @@ static struct platform_device eth_port1_device = {
 
 static struct platform_device *mv643xx_eth_pd_devs[] __initdata = {
 	&mv643xx_eth_shared_device,
+	&mv643xx_eth_mvmdio_device,
 	&eth_port1_device,
 };
 
diff --git a/arch/powerpc/sysdev/mv64x60_dev.c b/arch/powerpc/sysdev/mv64x60_dev.c
index 0f6af41..630cea3 100644
--- a/arch/powerpc/sysdev/mv64x60_dev.c
+++ b/arch/powerpc/sysdev/mv64x60_dev.c
@@ -214,15 +214,25 @@ static struct platform_device * __init mv64x60_eth_register_shared_pdev(
 						struct device_node *np, int id)
 {
 	struct platform_device *pdev;
-	struct resource r[1];
+	struct resource r[2];
 	int err;
 
 	err = of_address_to_resource(np, 0, &r[0]);
 	if (err)
 		return ERR_PTR(err);
 
+	/* register an orion mdio bus driver */
+	r[1].start = r[0].start + 0x4;
+	r[1].end = r[0].start + 0x84 - 1;
+	r[1].flags = IORESOURCE_MEM;
+
+	pdev = platform_device_register_simple("orion-mdio", id, &r[1], 1);
+	if (!pdev)
+		return pdev;
+
 	pdev = platform_device_register_simple(MV643XX_ETH_SHARED_NAME, id,
-					       r, 1);
+					       &r[0], 1);
+
 	return pdev;
 }
 
diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index edfba93..df06061 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -23,6 +23,7 @@ config MV643XX_ETH
 	depends on (MV64X60 || PPC32 || PLAT_ORION) && INET
 	select INET_LRO
 	select PHYLIB
+	select MVMDIO
 	---help---
 	  This driver supports the gigabit ethernet MACs in the
 	  Marvell Discovery PPC/MIPS chipset family (MV643XX) and
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index d1ecf4b..df04bee 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -69,14 +69,6 @@ static char mv643xx_eth_driver_version[] = "1.4";
  * Registers shared between all ports.
  */
 #define PHY_ADDR			0x0000
-#define SMI_REG				0x0004
-#define  SMI_BUSY			0x10000000
-#define  SMI_READ_VALID			0x08000000
-#define  SMI_OPCODE_READ		0x04000000
-#define  SMI_OPCODE_WRITE		0x00000000
-#define ERR_INT_CAUSE			0x0080
-#define  ERR_INT_SMI_DONE		0x00000010
-#define ERR_INT_MASK			0x0084
 #define WINDOW_BASE(w)			(0x0200 + ((w) << 3))
 #define WINDOW_SIZE(w)			(0x0204 + ((w) << 3))
 #define WINDOW_REMAP_HIGH(w)		(0x0280 + ((w) << 2))
@@ -266,25 +258,6 @@ struct mv643xx_eth_shared_private {
 	void __iomem *base;
 
 	/*
-	 * Points at the right SMI instance to use.
-	 */
-	struct mv643xx_eth_shared_private *smi;
-
-	/*
-	 * Provides access to local SMI interface.
-	 */
-	struct mii_bus *smi_bus;
-
-	/*
-	 * If we have access to the error interrupt pin (which is
-	 * somewhat misnamed as it not only reflects internal errors
-	 * but also reflects SMI completion), use that to wait for
-	 * SMI access completion instead of polling the SMI busy bit.
-	 */
-	int err_interrupt;
-	wait_queue_head_t smi_busy_wait;
-
-	/*
 	 * Per-port MBUS window access register value.
 	 */
 	u32 win_protect;
@@ -1122,97 +1095,6 @@ out_write:
 	wrlp(mp, PORT_SERIAL_CONTROL, pscr);
 }
 
-static irqreturn_t mv643xx_eth_err_irq(int irq, void *dev_id)
-{
-	struct mv643xx_eth_shared_private *msp = dev_id;
-
-	if (readl(msp->base + ERR_INT_CAUSE) & ERR_INT_SMI_DONE) {
-		writel(~ERR_INT_SMI_DONE, msp->base + ERR_INT_CAUSE);
-		wake_up(&msp->smi_busy_wait);
-		return IRQ_HANDLED;
-	}
-
-	return IRQ_NONE;
-}
-
-static int smi_is_done(struct mv643xx_eth_shared_private *msp)
-{
-	return !(readl(msp->base + SMI_REG) & SMI_BUSY);
-}
-
-static int smi_wait_ready(struct mv643xx_eth_shared_private *msp)
-{
-	if (msp->err_interrupt == NO_IRQ) {
-		int i;
-
-		for (i = 0; !smi_is_done(msp); i++) {
-			if (i == 10)
-				return -ETIMEDOUT;
-			msleep(10);
-		}
-
-		return 0;
-	}
-
-	if (!smi_is_done(msp)) {
-		wait_event_timeout(msp->smi_busy_wait, smi_is_done(msp),
-				   msecs_to_jiffies(100));
-		if (!smi_is_done(msp))
-			return -ETIMEDOUT;
-	}
-
-	return 0;
-}
-
-static int smi_bus_read(struct mii_bus *bus, int addr, int reg)
-{
-	struct mv643xx_eth_shared_private *msp = bus->priv;
-	void __iomem *smi_reg = msp->base + SMI_REG;
-	int ret;
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	writel(SMI_OPCODE_READ | (reg << 21) | (addr << 16), smi_reg);
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	ret = readl(smi_reg);
-	if (!(ret & SMI_READ_VALID)) {
-		pr_warn("SMI bus read not valid\n");
-		return -ENODEV;
-	}
-
-	return ret & 0xffff;
-}
-
-static int smi_bus_write(struct mii_bus *bus, int addr, int reg, u16 val)
-{
-	struct mv643xx_eth_shared_private *msp = bus->priv;
-	void __iomem *smi_reg = msp->base + SMI_REG;
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	writel(SMI_OPCODE_WRITE | (reg << 21) |
-		(addr << 16) | (val & 0xffff), smi_reg);
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	return 0;
-}
-
-
 /* statistics ***************************************************************/
 static struct net_device_stats *mv643xx_eth_get_stats(struct net_device *dev)
 {
@@ -2688,47 +2570,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 		goto out_free;
 
 	/*
-	 * Set up and register SMI bus.
-	 */
-	if (pd == NULL || pd->shared_smi == NULL) {
-		msp->smi_bus = mdiobus_alloc();
-		if (msp->smi_bus == NULL)
-			goto out_unmap;
-
-		msp->smi_bus->priv = msp;
-		msp->smi_bus->name = "mv643xx_eth smi";
-		msp->smi_bus->read = smi_bus_read;
-		msp->smi_bus->write = smi_bus_write,
-		snprintf(msp->smi_bus->id, MII_BUS_ID_SIZE, "%s-%d",
-			pdev->name, pdev->id);
-		msp->smi_bus->parent = &pdev->dev;
-		msp->smi_bus->phy_mask = 0xffffffff;
-		if (mdiobus_register(msp->smi_bus) < 0)
-			goto out_free_mii_bus;
-		msp->smi = msp;
-	} else {
-		msp->smi = platform_get_drvdata(pd->shared_smi);
-	}
-
-	msp->err_interrupt = NO_IRQ;
-	init_waitqueue_head(&msp->smi_busy_wait);
-
-	/*
-	 * Check whether the error interrupt is hooked up.
-	 */
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (res != NULL) {
-		int err;
-
-		err = request_irq(res->start, mv643xx_eth_err_irq,
-				  IRQF_SHARED, "mv643xx_eth", msp);
-		if (!err) {
-			writel(ERR_INT_SMI_DONE, msp->base + ERR_INT_MASK);
-			msp->err_interrupt = res->start;
-		}
-	}
-
-	/*
 	 * (Re-)program MBUS remapping windows if we are asked to.
 	 */
 	dram = mv_mbus_dram_info();
@@ -2743,10 +2584,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 
 	return 0;
 
-out_free_mii_bus:
-	mdiobus_free(msp->smi_bus);
-out_unmap:
-	iounmap(msp->base);
 out_free:
 	kfree(msp);
 out:
@@ -2756,14 +2593,7 @@ out:
 static int mv643xx_eth_shared_remove(struct platform_device *pdev)
 {
 	struct mv643xx_eth_shared_private *msp = platform_get_drvdata(pdev);
-	struct mv643xx_eth_shared_platform_data *pd = pdev->dev.platform_data;
 
-	if (pd == NULL || pd->shared_smi == NULL) {
-		mdiobus_unregister(msp->smi_bus);
-		mdiobus_free(msp->smi_bus);
-	}
-	if (msp->err_interrupt != NO_IRQ)
-		free_irq(msp->err_interrupt, msp);
 	iounmap(msp->base);
 	kfree(msp);
 
@@ -2829,11 +2659,11 @@ static void set_params(struct mv643xx_eth_private *mp,
 static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
 				   int phy_addr)
 {
-	struct mii_bus *bus = mp->shared->smi->smi_bus;
 	struct phy_device *phydev;
 	int start;
 	int num;
 	int i;
+	char phy_id[MII_BUS_ID_SIZE + 3];
 
 	if (phy_addr == MV643XX_ETH_PHY_ADDR_DEFAULT) {
 		start = phy_addr_get(mp) & 0x1f;
@@ -2843,17 +2673,19 @@ static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
 		num = 1;
 	}
 
+	/* Attempt to connect to the PHY using orion-mdio */
 	phydev = NULL;
 	for (i = 0; i < num; i++) {
 		int addr = (start + i) & 0x1f;
 
-		if (bus->phy_map[addr] == NULL)
-			mdiobus_scan(bus, addr);
+		snprintf(phy_id, sizeof(phy_id), PHY_ID_FMT,
+				"orion-mdio-mii", addr);
 
-		if (phydev == NULL) {
-			phydev = bus->phy_map[addr];
-			if (phydev != NULL)
-				phy_addr_set(mp, addr);
+		phydev = phy_connect(mp->dev, phy_id, NULL,
+				PHY_INTERFACE_MODE_GMII);
+		if (!IS_ERR(phydev)) {
+			phy_addr_set(mp, addr);
+			break;
 		}
 	}
 
diff --git a/include/linux/mv643xx_eth.h b/include/linux/mv643xx_eth.h
index 49258e0..141d395 100644
--- a/include/linux/mv643xx_eth.h
+++ b/include/linux/mv643xx_eth.h
@@ -19,7 +19,6 @@
 
 struct mv643xx_eth_shared_platform_data {
 	struct mbus_dram_target_info	*dram;
-	struct platform_device	*shared_smi;
 	/*
 	 * Max packet size for Tx IP/Layer 4 checksum, when set to 0, default
 	 * limit of 9KiB will be used.
-- 
1.7.10.4

Re: [PATCH] mv643xx_eth: convert to use the Marvell Orion MDIO driver

[Jason Cooper]

Florian,

On Thu, Mar 14, 2013 at 07:08:32PM +0100, Florian Fainelli wrote:
> This patch converts the Marvell MV643XX ethernet driver to use the
> Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
> registering the Marvell MV643XX ethernet driver are also updated to
> register a Marvell Orion MDIO driver. This driver voluntarily overlaps
> with the Marvell Ethernet shared registers because it will use a subset
> of this shared register (shared_base + 0x4 - shared_base + 0x84). The
> Ethernet driver is also updated to look up for a PHY device using the
> Orion MDIO bus driver.
> 
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> ---
>  arch/arm/plat-orion/common.c               |   97 +++++++++++----
>  arch/powerpc/platforms/chrp/pegasos_eth.c  |   20 +++
>  arch/powerpc/sysdev/mv64x60_dev.c          |   14 ++-
>  drivers/net/ethernet/marvell/Kconfig       |    1 +
>  drivers/net/ethernet/marvell/mv643xx_eth.c |  186 ++--------------------------
>  include/linux/mv643xx_eth.h                |    1 -
>  6 files changed, 117 insertions(+), 202 deletions(-)

Isn't this the old version of 4/4 ?

thx,

Jason.

> 
> diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
> index 2d4b641..dc4345e 100644
> --- a/arch/arm/plat-orion/common.c
> +++ b/arch/arm/plat-orion/common.c
> @@ -238,6 +238,7 @@ static __init void ge_complete(
>  	struct mv643xx_eth_shared_platform_data *orion_ge_shared_data,
>  	struct resource *orion_ge_resource, unsigned long irq,
>  	struct platform_device *orion_ge_shared,
> +	struct platform_device *orion_ge_mvmdio,
>  	struct mv643xx_eth_platform_data *eth_data,
>  	struct platform_device *orion_ge)
>  {
> @@ -247,6 +248,8 @@ static __init void ge_complete(
>  	orion_ge->dev.platform_data = eth_data;
>  
>  	platform_device_register(orion_ge_shared);
> +	if (orion_ge_mvmdio)
> +		platform_device_register(orion_ge_mvmdio);
>  	platform_device_register(orion_ge);
>  }
>  
> @@ -258,8 +261,6 @@ struct mv643xx_eth_shared_platform_data orion_ge00_shared_data;
>  static struct resource orion_ge00_shared_resources[] = {
>  	{
>  		.name	= "ge00 base",
> -	}, {
> -		.name	= "ge00 err irq",
>  	},
>  };
>  
> @@ -271,6 +272,19 @@ static struct platform_device orion_ge00_shared = {
>  	},
>  };
>  
> +static struct resource orion_ge00_mvmdio_resources[] = {
> +	{
> +		.name	= "ge00 mvmdio base",
> +	}, {
> +		.name	= "ge00 mvmdio irr irq",
> +	},
> +};
> +
> +static struct platform_device orion_ge00_mvmdio = {
> +	.name		= "orion-mdio",
> +	.id		= 0,
> +};
> +
>  static struct resource orion_ge00_resources[] = {
>  	{
>  		.name	= "ge00 irq",
> @@ -295,26 +309,25 @@ void __init orion_ge00_init(struct mv643xx_eth_platform_data *eth_data,
>  			    unsigned int tx_csum_limit)
>  {
>  	fill_resources(&orion_ge00_shared, orion_ge00_shared_resources,
> -		       mapbase + 0x2000, SZ_16K - 1, irq_err);
> +		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
> +	fill_resources(&orion_ge00_mvmdio, orion_ge00_mvmdio_resources,
> +			mapbase + 0x2004, 0x84 - 1, irq_err);
>  	orion_ge00_shared_data.tx_csum_limit = tx_csum_limit;
>  	ge_complete(&orion_ge00_shared_data,
>  		    orion_ge00_resources, irq, &orion_ge00_shared,
> +		    &orion_ge00_mvmdio,
>  		    eth_data, &orion_ge00);
>  }
>  
>  /*****************************************************************************
>   * GE01
>   ****************************************************************************/
> -struct mv643xx_eth_shared_platform_data orion_ge01_shared_data = {
> -	.shared_smi	= &orion_ge00_shared,
> -};
> +struct mv643xx_eth_shared_platform_data orion_ge01_shared_data;
>  
>  static struct resource orion_ge01_shared_resources[] = {
>  	{
>  		.name	= "ge01 base",
> -	}, {
> -		.name	= "ge01 err irq",
> -	},
> +	}
>  };
>  
>  static struct platform_device orion_ge01_shared = {
> @@ -325,6 +338,19 @@ static struct platform_device orion_ge01_shared = {
>  	},
>  };
>  
> +static struct resource orion_ge01_mvmdio_resources[] = {
> +	{
> +		.name	= "ge01 mdio base",
> +	}, {
> +		.name	= "ge01 mdio err irq",
> +	},
> +};
> +
> +static struct platform_device orion_ge01_mvmdio = {
> +	.name		= "orion-mdio",
> +	.id		= 1,
> +};
> +
>  static struct resource orion_ge01_resources[] = {
>  	{
>  		.name	= "ge01 irq",
> @@ -349,26 +375,25 @@ void __init orion_ge01_init(struct mv643xx_eth_platform_data *eth_data,
>  			    unsigned int tx_csum_limit)
>  {
>  	fill_resources(&orion_ge01_shared, orion_ge01_shared_resources,
> -		       mapbase + 0x2000, SZ_16K - 1, irq_err);
> +		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
> +	fill_resources(&orion_ge01_mvmdio, orion_ge01_mvmdio_resources,
> +			mapbase + 0x2004, 0x84 - 1, irq_err);
>  	orion_ge01_shared_data.tx_csum_limit = tx_csum_limit;
>  	ge_complete(&orion_ge01_shared_data,
>  		    orion_ge01_resources, irq, &orion_ge01_shared,
> +		    NULL,
>  		    eth_data, &orion_ge01);
>  }
>  
>  /*****************************************************************************
>   * GE10
>   ****************************************************************************/
> -struct mv643xx_eth_shared_platform_data orion_ge10_shared_data = {
> -	.shared_smi	= &orion_ge00_shared,
> -};
> +struct mv643xx_eth_shared_platform_data orion_ge10_shared_data;
>  
>  static struct resource orion_ge10_shared_resources[] = {
>  	{
>  		.name	= "ge10 base",
> -	}, {
> -		.name	= "ge10 err irq",
> -	},
> +	}
>  };
>  
>  static struct platform_device orion_ge10_shared = {
> @@ -379,6 +404,19 @@ static struct platform_device orion_ge10_shared = {
>  	},
>  };
>  
> +static struct resource orion_ge10_mvmdio_resources[] = {
> +	{
> +		.name	= "ge10 mvmdio base",
> +	}, {
> +		.name	= "ge10 mvmdio err irq",
> +	},
> +};
> +
> +static struct platform_device orion_ge10_mvmdio = {
> +	.name		= "orion-mdio",
> +	.id		= 1,
> +};
> +
>  static struct resource orion_ge10_resources[] = {
>  	{
>  		.name	= "ge10 irq",
> @@ -403,23 +441,22 @@ void __init orion_ge10_init(struct mv643xx_eth_platform_data *eth_data,
>  {
>  	fill_resources(&orion_ge10_shared, orion_ge10_shared_resources,
>  		       mapbase + 0x2000, SZ_16K - 1, irq_err);
> +	fill_resources(&orion_ge10_mvmdio, orion_ge10_mvmdio_resources,
> +		       mapbase + 0x2004, 0x84 - 1, irq_err);
>  	ge_complete(&orion_ge10_shared_data,
>  		    orion_ge10_resources, irq, &orion_ge10_shared,
> +		    &orion_ge10_mvmdio,
>  		    eth_data, &orion_ge10);
>  }
>  
>  /*****************************************************************************
>   * GE11
>   ****************************************************************************/
> -struct mv643xx_eth_shared_platform_data orion_ge11_shared_data = {
> -	.shared_smi	= &orion_ge00_shared,
> -};
> +struct mv643xx_eth_shared_platform_data orion_ge11_shared_data;
>  
>  static struct resource orion_ge11_shared_resources[] = {
>  	{
>  		.name	= "ge11 base",
> -	}, {
> -		.name	= "ge11 err irq",
>  	},
>  };
>  
> @@ -431,6 +468,19 @@ static struct platform_device orion_ge11_shared = {
>  	},
>  };
>  
> +static struct resource orion_ge11_mvmdio_resources[] = {
> +	{
> +		.name	= "ge11 mvmdio base",
> +	}, {
> +		.name	= "ge11 mvmdio err irq",
> +	},
> +};
> +
> +static struct platform_device orion_ge11_mvmdio = {
> +	.name		= "orion-mdio",
> +	.id		= 1,
> +};
> +
>  static struct resource orion_ge11_resources[] = {
>  	{
>  		.name	= "ge11 irq",
> @@ -454,9 +504,12 @@ void __init orion_ge11_init(struct mv643xx_eth_platform_data *eth_data,
>  			    unsigned long irq_err)
>  {
>  	fill_resources(&orion_ge11_shared, orion_ge11_shared_resources,
> -		       mapbase + 0x2000, SZ_16K - 1, irq_err);
> +		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
> +	fill_resources(&orion_ge11_mvmdio, orion_ge11_mvmdio_resources,
> +		       mapbase + 0x2004, 0x84 - 1, irq_err);
>  	ge_complete(&orion_ge11_shared_data,
>  		    orion_ge11_resources, irq, &orion_ge11_shared,
> +		    NULL,
>  		    eth_data, &orion_ge11);
>  }
>  
> diff --git a/arch/powerpc/platforms/chrp/pegasos_eth.c b/arch/powerpc/platforms/chrp/pegasos_eth.c
> index 039fc8e..a671508 100644
> --- a/arch/powerpc/platforms/chrp/pegasos_eth.c
> +++ b/arch/powerpc/platforms/chrp/pegasos_eth.c
> @@ -47,6 +47,25 @@ static struct platform_device mv643xx_eth_shared_device = {
>  	.resource	= mv643xx_eth_shared_resources,
>  };
>  
> +/*
> + * The orion mdio driver only covers shared + 0x4 up to shared + 0x84 - 1
> + */
> +static struct resource mv643xx_eth_mvmdio_resources[] = {
> +	[0] = {
> +		.name	= "ethernet mdio base",
> +		.start	= 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x4,
> +		.end	= 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x83,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +};
> +
> +static struct platform_device mv643xx_eth_mvmdio_device = {
> +	.name		= "orion-mdio",
> +	.id		= 0,
> +	.num_resources	= ARRAY_SIZE(mv643xx_eth_mvmdio_resources),
> +	.resource	= mv643xx_eth_shared_resources,
> +};
> +
>  static struct resource mv643xx_eth_port1_resources[] = {
>  	[0] = {
>  		.name	= "eth port1 irq",
> @@ -82,6 +101,7 @@ static struct platform_device eth_port1_device = {
>  
>  static struct platform_device *mv643xx_eth_pd_devs[] __initdata = {
>  	&mv643xx_eth_shared_device,
> +	&mv643xx_eth_mvmdio_device,
>  	&eth_port1_device,
>  };
>  
> diff --git a/arch/powerpc/sysdev/mv64x60_dev.c b/arch/powerpc/sysdev/mv64x60_dev.c
> index 0f6af41..630cea3 100644
> --- a/arch/powerpc/sysdev/mv64x60_dev.c
> +++ b/arch/powerpc/sysdev/mv64x60_dev.c
> @@ -214,15 +214,25 @@ static struct platform_device * __init mv64x60_eth_register_shared_pdev(
>  						struct device_node *np, int id)
>  {
>  	struct platform_device *pdev;
> -	struct resource r[1];
> +	struct resource r[2];
>  	int err;
>  
>  	err = of_address_to_resource(np, 0, &r[0]);
>  	if (err)
>  		return ERR_PTR(err);
>  
> +	/* register an orion mdio bus driver */
> +	r[1].start = r[0].start + 0x4;
> +	r[1].end = r[0].start + 0x84 - 1;
> +	r[1].flags = IORESOURCE_MEM;
> +
> +	pdev = platform_device_register_simple("orion-mdio", id, &r[1], 1);
> +	if (!pdev)
> +		return pdev;
> +
>  	pdev = platform_device_register_simple(MV643XX_ETH_SHARED_NAME, id,
> -					       r, 1);
> +					       &r[0], 1);
> +
>  	return pdev;
>  }
>  
> diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
> index edfba93..df06061 100644
> --- a/drivers/net/ethernet/marvell/Kconfig
> +++ b/drivers/net/ethernet/marvell/Kconfig
> @@ -23,6 +23,7 @@ config MV643XX_ETH
>  	depends on (MV64X60 || PPC32 || PLAT_ORION) && INET
>  	select INET_LRO
>  	select PHYLIB
> +	select MVMDIO
>  	---help---
>  	  This driver supports the gigabit ethernet MACs in the
>  	  Marvell Discovery PPC/MIPS chipset family (MV643XX) and
> diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
> index d1ecf4b..df04bee 100644
> --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
> +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
> @@ -69,14 +69,6 @@ static char mv643xx_eth_driver_version[] = "1.4";
>   * Registers shared between all ports.
>   */
>  #define PHY_ADDR			0x0000
> -#define SMI_REG				0x0004
> -#define  SMI_BUSY			0x10000000
> -#define  SMI_READ_VALID			0x08000000
> -#define  SMI_OPCODE_READ		0x04000000
> -#define  SMI_OPCODE_WRITE		0x00000000
> -#define ERR_INT_CAUSE			0x0080
> -#define  ERR_INT_SMI_DONE		0x00000010
> -#define ERR_INT_MASK			0x0084
>  #define WINDOW_BASE(w)			(0x0200 + ((w) << 3))
>  #define WINDOW_SIZE(w)			(0x0204 + ((w) << 3))
>  #define WINDOW_REMAP_HIGH(w)		(0x0280 + ((w) << 2))
> @@ -266,25 +258,6 @@ struct mv643xx_eth_shared_private {
>  	void __iomem *base;
>  
>  	/*
> -	 * Points at the right SMI instance to use.
> -	 */
> -	struct mv643xx_eth_shared_private *smi;
> -
> -	/*
> -	 * Provides access to local SMI interface.
> -	 */
> -	struct mii_bus *smi_bus;
> -
> -	/*
> -	 * If we have access to the error interrupt pin (which is
> -	 * somewhat misnamed as it not only reflects internal errors
> -	 * but also reflects SMI completion), use that to wait for
> -	 * SMI access completion instead of polling the SMI busy bit.
> -	 */
> -	int err_interrupt;
> -	wait_queue_head_t smi_busy_wait;
> -
> -	/*
>  	 * Per-port MBUS window access register value.
>  	 */
>  	u32 win_protect;
> @@ -1122,97 +1095,6 @@ out_write:
>  	wrlp(mp, PORT_SERIAL_CONTROL, pscr);
>  }
>  
> -static irqreturn_t mv643xx_eth_err_irq(int irq, void *dev_id)
> -{
> -	struct mv643xx_eth_shared_private *msp = dev_id;
> -
> -	if (readl(msp->base + ERR_INT_CAUSE) & ERR_INT_SMI_DONE) {
> -		writel(~ERR_INT_SMI_DONE, msp->base + ERR_INT_CAUSE);
> -		wake_up(&msp->smi_busy_wait);
> -		return IRQ_HANDLED;
> -	}
> -
> -	return IRQ_NONE;
> -}
> -
> -static int smi_is_done(struct mv643xx_eth_shared_private *msp)
> -{
> -	return !(readl(msp->base + SMI_REG) & SMI_BUSY);
> -}
> -
> -static int smi_wait_ready(struct mv643xx_eth_shared_private *msp)
> -{
> -	if (msp->err_interrupt == NO_IRQ) {
> -		int i;
> -
> -		for (i = 0; !smi_is_done(msp); i++) {
> -			if (i == 10)
> -				return -ETIMEDOUT;
> -			msleep(10);
> -		}
> -
> -		return 0;
> -	}
> -
> -	if (!smi_is_done(msp)) {
> -		wait_event_timeout(msp->smi_busy_wait, smi_is_done(msp),
> -				   msecs_to_jiffies(100));
> -		if (!smi_is_done(msp))
> -			return -ETIMEDOUT;
> -	}
> -
> -	return 0;
> -}
> -
> -static int smi_bus_read(struct mii_bus *bus, int addr, int reg)
> -{
> -	struct mv643xx_eth_shared_private *msp = bus->priv;
> -	void __iomem *smi_reg = msp->base + SMI_REG;
> -	int ret;
> -
> -	if (smi_wait_ready(msp)) {
> -		pr_warn("SMI bus busy timeout\n");
> -		return -ETIMEDOUT;
> -	}
> -
> -	writel(SMI_OPCODE_READ | (reg << 21) | (addr << 16), smi_reg);
> -
> -	if (smi_wait_ready(msp)) {
> -		pr_warn("SMI bus busy timeout\n");
> -		return -ETIMEDOUT;
> -	}
> -
> -	ret = readl(smi_reg);
> -	if (!(ret & SMI_READ_VALID)) {
> -		pr_warn("SMI bus read not valid\n");
> -		return -ENODEV;
> -	}
> -
> -	return ret & 0xffff;
> -}
> -
> -static int smi_bus_write(struct mii_bus *bus, int addr, int reg, u16 val)
> -{
> -	struct mv643xx_eth_shared_private *msp = bus->priv;
> -	void __iomem *smi_reg = msp->base + SMI_REG;
> -
> -	if (smi_wait_ready(msp)) {
> -		pr_warn("SMI bus busy timeout\n");
> -		return -ETIMEDOUT;
> -	}
> -
> -	writel(SMI_OPCODE_WRITE | (reg << 21) |
> -		(addr << 16) | (val & 0xffff), smi_reg);
> -
> -	if (smi_wait_ready(msp)) {
> -		pr_warn("SMI bus busy timeout\n");
> -		return -ETIMEDOUT;
> -	}
> -
> -	return 0;
> -}
> -
> -
>  /* statistics ***************************************************************/
>  static struct net_device_stats *mv643xx_eth_get_stats(struct net_device *dev)
>  {
> @@ -2688,47 +2570,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
>  		goto out_free;
>  
>  	/*
> -	 * Set up and register SMI bus.
> -	 */
> -	if (pd == NULL || pd->shared_smi == NULL) {
> -		msp->smi_bus = mdiobus_alloc();
> -		if (msp->smi_bus == NULL)
> -			goto out_unmap;
> -
> -		msp->smi_bus->priv = msp;
> -		msp->smi_bus->name = "mv643xx_eth smi";
> -		msp->smi_bus->read = smi_bus_read;
> -		msp->smi_bus->write = smi_bus_write,
> -		snprintf(msp->smi_bus->id, MII_BUS_ID_SIZE, "%s-%d",
> -			pdev->name, pdev->id);
> -		msp->smi_bus->parent = &pdev->dev;
> -		msp->smi_bus->phy_mask = 0xffffffff;
> -		if (mdiobus_register(msp->smi_bus) < 0)
> -			goto out_free_mii_bus;
> -		msp->smi = msp;
> -	} else {
> -		msp->smi = platform_get_drvdata(pd->shared_smi);
> -	}
> -
> -	msp->err_interrupt = NO_IRQ;
> -	init_waitqueue_head(&msp->smi_busy_wait);
> -
> -	/*
> -	 * Check whether the error interrupt is hooked up.
> -	 */
> -	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> -	if (res != NULL) {
> -		int err;
> -
> -		err = request_irq(res->start, mv643xx_eth_err_irq,
> -				  IRQF_SHARED, "mv643xx_eth", msp);
> -		if (!err) {
> -			writel(ERR_INT_SMI_DONE, msp->base + ERR_INT_MASK);
> -			msp->err_interrupt = res->start;
> -		}
> -	}
> -
> -	/*
>  	 * (Re-)program MBUS remapping windows if we are asked to.
>  	 */
>  	dram = mv_mbus_dram_info();
> @@ -2743,10 +2584,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> -out_free_mii_bus:
> -	mdiobus_free(msp->smi_bus);
> -out_unmap:
> -	iounmap(msp->base);
>  out_free:
>  	kfree(msp);
>  out:
> @@ -2756,14 +2593,7 @@ out:
>  static int mv643xx_eth_shared_remove(struct platform_device *pdev)
>  {
>  	struct mv643xx_eth_shared_private *msp = platform_get_drvdata(pdev);
> -	struct mv643xx_eth_shared_platform_data *pd = pdev->dev.platform_data;
>  
> -	if (pd == NULL || pd->shared_smi == NULL) {
> -		mdiobus_unregister(msp->smi_bus);
> -		mdiobus_free(msp->smi_bus);
> -	}
> -	if (msp->err_interrupt != NO_IRQ)
> -		free_irq(msp->err_interrupt, msp);
>  	iounmap(msp->base);
>  	kfree(msp);
>  
> @@ -2829,11 +2659,11 @@ static void set_params(struct mv643xx_eth_private *mp,
>  static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
>  				   int phy_addr)
>  {
> -	struct mii_bus *bus = mp->shared->smi->smi_bus;
>  	struct phy_device *phydev;
>  	int start;
>  	int num;
>  	int i;
> +	char phy_id[MII_BUS_ID_SIZE + 3];
>  
>  	if (phy_addr == MV643XX_ETH_PHY_ADDR_DEFAULT) {
>  		start = phy_addr_get(mp) & 0x1f;
> @@ -2843,17 +2673,19 @@ static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
>  		num = 1;
>  	}
>  
> +	/* Attempt to connect to the PHY using orion-mdio */
>  	phydev = NULL;
>  	for (i = 0; i < num; i++) {
>  		int addr = (start + i) & 0x1f;
>  
> -		if (bus->phy_map[addr] == NULL)
> -			mdiobus_scan(bus, addr);
> +		snprintf(phy_id, sizeof(phy_id), PHY_ID_FMT,
> +				"orion-mdio-mii", addr);
>  
> -		if (phydev == NULL) {
> -			phydev = bus->phy_map[addr];
> -			if (phydev != NULL)
> -				phy_addr_set(mp, addr);
> +		phydev = phy_connect(mp->dev, phy_id, NULL,
> +				PHY_INTERFACE_MODE_GMII);
> +		if (!IS_ERR(phydev)) {
> +			phy_addr_set(mp, addr);
> +			break;
>  		}
>  	}
>  
> diff --git a/include/linux/mv643xx_eth.h b/include/linux/mv643xx_eth.h
> index 49258e0..141d395 100644
> --- a/include/linux/mv643xx_eth.h
> +++ b/include/linux/mv643xx_eth.h
> @@ -19,7 +19,6 @@
>  
>  struct mv643xx_eth_shared_platform_data {
>  	struct mbus_dram_target_info	*dram;
> -	struct platform_device	*shared_smi;
>  	/*
>  	 * Max packet size for Tx IP/Layer 4 checksum, when set to 0, default
>  	 * limit of 9KiB will be used.
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mv643xx_eth: convert to use the Marvell Orion MDIO driver

[Florian Fainelli]

Hello Jason,

Le 14/03/2013 20:02, Jason Cooper a écrit :
> Florian,
>
> On Thu, Mar 14, 2013 at 07:08:32PM +0100, Florian Fainelli wrote:
>> This patch converts the Marvell MV643XX ethernet driver to use the
>> Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
>> registering the Marvell MV643XX ethernet driver are also updated to
>> register a Marvell Orion MDIO driver. This driver voluntarily overlaps
>> with the Marvell Ethernet shared registers because it will use a subset
>> of this shared register (shared_base + 0x4 - shared_base + 0x84). The
>> Ethernet driver is also updated to look up for a PHY device using the
>> Orion MDIO bus driver.
>>
>> Signed-off-by: Florian Fainelli <florian@openwrt.org>
>> ---
>>   arch/arm/plat-orion/common.c               |   97 +++++++++++----
>>   arch/powerpc/platforms/chrp/pegasos_eth.c  |   20 +++
>>   arch/powerpc/sysdev/mv64x60_dev.c          |   14 ++-
>>   drivers/net/ethernet/marvell/Kconfig       |    1 +
>>   drivers/net/ethernet/marvell/mv643xx_eth.c |  186 ++--------------------------
>>   include/linux/mv643xx_eth.h                |    1 -
>>   6 files changed, 117 insertions(+), 202 deletions(-)
>
> Isn't this the old version of 4/4 ?

No this really is v2, but I reformatted just that one and sent it 
without the v2 changelog, you should have received the proper version a 
couple minutes later once I realized my mistake.
--
Florian

Re: [PATCH] mv643xx_eth: convert to use the Marvell Orion MDIO driver

[Jason Cooper]

On Thu, Mar 14, 2013 at 08:32:57PM +0100, Florian Fainelli wrote:
> Hello Jason,
> 
> Le 14/03/2013 20:02, Jason Cooper a écrit :
> >Florian,
> >
> >On Thu, Mar 14, 2013 at 07:08:32PM +0100, Florian Fainelli wrote:
> >>This patch converts the Marvell MV643XX ethernet driver to use the
> >>Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
> >>registering the Marvell MV643XX ethernet driver are also updated to
> >>register a Marvell Orion MDIO driver. This driver voluntarily overlaps
> >>with the Marvell Ethernet shared registers because it will use a subset
> >>of this shared register (shared_base + 0x4 - shared_base + 0x84). The
> >>Ethernet driver is also updated to look up for a PHY device using the
> >>Orion MDIO bus driver.
> >>
> >>Signed-off-by: Florian Fainelli <florian@openwrt.org>
> >>---
> >>  arch/arm/plat-orion/common.c               |   97 +++++++++++----
> >>  arch/powerpc/platforms/chrp/pegasos_eth.c  |   20 +++
> >>  arch/powerpc/sysdev/mv64x60_dev.c          |   14 ++-
> >>  drivers/net/ethernet/marvell/Kconfig       |    1 +
> >>  drivers/net/ethernet/marvell/mv643xx_eth.c |  186 ++--------------------------
> >>  include/linux/mv643xx_eth.h                |    1 -
> >>  6 files changed, 117 insertions(+), 202 deletions(-)
> >
> >Isn't this the old version of 4/4 ?
> 
> No this really is v2, but I reformatted just that one and sent it
> without the v2 changelog, you should have received the proper
> version a couple minutes later once I realized my mistake.

Yes, I received both, just making sure.

thx,

Jason.

[PATCH 2/4 v2] net: mvmdio: rename base register cookie from smireg to regs

[Florian Fainelli]

This patch renames the base register cookie in the mvmdio drive from
"smireg" to "regs" since a subsequent patch is going to use an ioremap()
cookie whose size is larger than a single register of 4 bytes. No
functionnal code change introduced.

Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
Changes since v1:
- added Thomas Acked-by tag

 drivers/net/ethernet/marvell/mvmdio.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 8a182f1..595deea 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -38,7 +38,7 @@
 
 struct orion_mdio_dev {
 	struct mutex lock;
-	void __iomem *smireg;
+	void __iomem *regs;
 };
 
 /* Wait for the SMI unit to be ready for another operation
@@ -51,7 +51,7 @@ static int orion_mdio_wait_ready(struct mii_bus *bus)
 
 	count = 0;
 	while (1) {
-		val = readl(dev->smireg);
+		val = readl(dev->regs);
 		if (!(val & MVMDIO_SMI_BUSY))
 			break;
 
@@ -86,12 +86,12 @@ static int orion_mdio_read(struct mii_bus *bus, int mii_id,
 	writel(((mii_id << MVMDIO_SMI_PHY_ADDR_SHIFT) |
 		(regnum << MVMDIO_SMI_PHY_REG_SHIFT)  |
 		MVMDIO_SMI_READ_OPERATION),
-	       dev->smireg);
+	       dev->regs);
 
 	/* Wait for the value to become available */
 	count = 0;
 	while (1) {
-		val = readl(dev->smireg);
+		val = readl(dev->regs);
 		if (val & MVMDIO_SMI_READ_VALID)
 			break;
 
@@ -128,7 +128,7 @@ static int orion_mdio_write(struct mii_bus *bus, int mii_id,
 		(regnum << MVMDIO_SMI_PHY_REG_SHIFT)  |
 		MVMDIO_SMI_WRITE_OPERATION            |
 		(value << MVMDIO_SMI_DATA_SHIFT)),
-	       dev->smireg);
+	       dev->regs);
 
 	mutex_unlock(&dev->lock);
 
@@ -177,8 +177,8 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		bus->irq[i] = PHY_POLL;
 
 	dev = bus->priv;
-	dev->smireg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
-	if (!dev->smireg) {
+	dev->regs = devm_ioremap(&pdev->dev, r->start, resource_size(r));
+	if (!dev->regs) {
 		dev_err(&pdev->dev, "Unable to remap SMI register\n");
 		kfree(bus->irq);
 		mdiobus_free(bus);
-- 
1.7.10.4

[PATCH 3/4 v2] net: mvmdio: enhance driver to support SMI error/done interrupts

[Florian Fainelli]

This patch enhances the "mvmdio" to support a SMI error/done interrupt
line which can be used along with a wait queue instead of doing
busy-waiting on the registers. This is a feature which is available in
the mv643xx_eth SMI code and thus reduces again the gap between the two.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
Chances since v1:
- always use orion_smi_is_done() helper
- make interrupt/non-interrupt code path entirely independant

 .../devicetree/bindings/net/marvell-orion-mdio.txt |    3 +
 drivers/net/ethernet/marvell/mvmdio.c              |   83 +++++++++++++++++---
 2 files changed, 74 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
index 34e7aaf..052b5f2 100644
--- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
+++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
@@ -9,6 +9,9 @@ Required properties:
 - compatible: "marvell,orion-mdio"
 - reg: address and length of the SMI register
 
+Optional properties:
+- interrupts: interrupt line number for the SMI error/done interrupt
+
 The child nodes of the MDIO driver are the individual PHY devices
 connected to this MDIO bus. They must have a "reg" property given the
 PHY address on the MDIO bus.
diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 595deea..7ac83de 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -24,9 +24,12 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/phy.h>
+#include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/sched.h>
+#include <linux/wait.h>
 
 #define MVMDIO_SMI_DATA_SHIFT              0
 #define MVMDIO_SMI_PHY_ADDR_SHIFT          16
@@ -35,33 +38,58 @@
 #define MVMDIO_SMI_WRITE_OPERATION         0
 #define MVMDIO_SMI_READ_VALID              BIT(27)
 #define MVMDIO_SMI_BUSY                    BIT(28)
+#define MVMDIO_ERR_INT_CAUSE		   0x007C
+#define  MVMDIO_ERR_INT_SMI_DONE	   0x00000010
+#define MVMDIO_ERR_INT_MASK		   0x0080
 
 struct orion_mdio_dev {
 	struct mutex lock;
 	void __iomem *regs;
+	/*
+	 * If we have access to the error interrupt pin (which is
+	 * somewhat misnamed as it not only reflects internal errors
+	 * but also reflects SMI completion), use that to wait for
+	 * SMI access completion instead of polling the SMI busy bit.
+	 */
+	int err_interrupt;
+	wait_queue_head_t smi_busy_wait;
 };
 
+static int orion_mdio_smi_is_done(struct orion_mdio_dev *dev)
+{
+	return !(readl(dev->regs) & MVMDIO_SMI_BUSY);
+}
+
 /* Wait for the SMI unit to be ready for another operation
  */
 static int orion_mdio_wait_ready(struct mii_bus *bus)
 {
 	struct orion_mdio_dev *dev = bus->priv;
 	int count;
-	u32 val;
 
-	count = 0;
-	while (1) {
-		val = readl(dev->regs);
-		if (!(val & MVMDIO_SMI_BUSY))
-			break;
+	if (dev->err_interrupt == NO_IRQ) {
+		count = 0;
+		while (1) {
+			if (orion_mdio_smi_is_done(dev))
+				break;
 
-		if (count > 100) {
-			dev_err(bus->parent, "Timeout: SMI busy for too long\n");
-			return -ETIMEDOUT;
-		}
+			if (count > 100) {
+				dev_err(bus->parent,
+					"Timeout: SMI busy for too long\n");
+				return -ETIMEDOUT;
+			}
 
-		udelay(10);
-		count++;
+			udelay(10);
+			count++;
+		}
+	} else {
+		if (!orion_mdio_smi_is_done(dev)) {
+			wait_event_timeout(dev->smi_busy_wait,
+				orion_mdio_smi_is_done(dev),
+				msecs_to_jiffies(100));
+			if (!orion_mdio_smi_is_done(dev))
+				return -ETIMEDOUT;
+		}
 	}
 
 	return 0;
@@ -140,6 +168,21 @@ static int orion_mdio_reset(struct mii_bus *bus)
 	return 0;
 }
 
+static irqreturn_t orion_mdio_err_irq(int irq, void *dev_id)
+{
+	struct orion_mdio_dev *dev = dev_id;
+
+	if (readl(dev->regs + MVMDIO_ERR_INT_CAUSE) &
+			MVMDIO_ERR_INT_SMI_DONE) {
+		writel(~MVMDIO_ERR_INT_SMI_DONE,
+				dev->regs + MVMDIO_ERR_INT_CAUSE);
+		wake_up(&dev->smi_busy_wait);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
 static int orion_mdio_probe(struct platform_device *pdev)
 {
 	struct resource *r;
@@ -185,6 +228,19 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	init_waitqueue_head(&dev->smi_busy_wait);
+
+	dev->err_interrupt = platform_get_irq(pdev, 0);
+	if (dev->err_interrupt != -ENXIO) {
+		ret = devm_request_irq(&pdev->dev, dev->err_interrupt,
+					orion_mdio_err_irq,
+					IRQF_SHARED, pdev->name, dev);
+		if (!ret)
+			writel(MVMDIO_ERR_INT_SMI_DONE,
+					dev->regs + MVMDIO_ERR_INT_MASK);
+	} else
+		dev->err_interrupt = NO_IRQ;
+
 	mutex_init(&dev->lock);
 
 	ret = mdiobus_register(bus);
@@ -203,6 +259,9 @@ static int orion_mdio_probe(struct platform_device *pdev)
 static int orion_mdio_remove(struct platform_device *pdev)
 {
 	struct mii_bus *bus = platform_get_drvdata(pdev);
+	struct orion_mdio_dev *dev = bus->priv;
+
+	writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
 	mdiobus_unregister(bus);
 	kfree(bus->irq);
 	mdiobus_free(bus);
-- 
1.7.10.4

Re: [PATCH 3/4 v2] net: mvmdio: enhance driver to support SMI error/done interrupts

[Russell King - ARM Linux]

On Thu, Mar 14, 2013 at 07:08:34PM +0100, Florian Fainelli wrote:
> +	if (dev->err_interrupt == NO_IRQ) {
...
> +	init_waitqueue_head(&dev->smi_busy_wait);
> +
> +	dev->err_interrupt = platform_get_irq(pdev, 0);
> +	if (dev->err_interrupt != -ENXIO) {
...
> +	} else
> +		dev->err_interrupt = NO_IRQ;


FYI, NO_IRQ is not supposed to be used anymore (we're supposed to be
removing it).  platform_get_irq() returns negative numbers for failure,
so why not test for < 0 in both the above tests, or maybe <= 0 (as
IRQ0 is also not supposed to be valid.)?

[PATCH 4/4 v2] mv643xx_eth: convert to use the Marvell Orion MDIO driver

[Florian Fainelli]

This patch converts the Marvell MV643XX ethernet driver to use the
Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
registering the Marvell MV643XX ethernet driver are also updated to
register a Marvell Orion MDIO driver. This driver voluntarily overlaps
with the Marvell Ethernet shared registers because it will use a subset
of this shared register (shared_base + 0x4 - shared_base + 0x84). The
Ethernet driver is also updated to look up for a PHY device using the
Orion MDIO bus driver.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
Changes since v1:
- removed one too many mdio bus registration for Orion5x/Kirkwood

 arch/arm/plat-orion/common.c               |   97 +++++++++++----
 arch/powerpc/platforms/chrp/pegasos_eth.c  |   20 +++
 arch/powerpc/sysdev/mv64x60_dev.c          |   14 ++-
 drivers/net/ethernet/marvell/Kconfig       |    1 +
 drivers/net/ethernet/marvell/mv643xx_eth.c |  186 ++--------------------------
 include/linux/mv643xx_eth.h                |    1 -
 6 files changed, 117 insertions(+), 202 deletions(-)

diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index 2d4b641..dc4345e 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -238,6 +238,7 @@ static __init void ge_complete(
 	struct mv643xx_eth_shared_platform_data *orion_ge_shared_data,
 	struct resource *orion_ge_resource, unsigned long irq,
 	struct platform_device *orion_ge_shared,
+	struct platform_device *orion_ge_mvmdio,
 	struct mv643xx_eth_platform_data *eth_data,
 	struct platform_device *orion_ge)
 {
@@ -247,6 +248,8 @@ static __init void ge_complete(
 	orion_ge->dev.platform_data = eth_data;
 
 	platform_device_register(orion_ge_shared);
+	if (orion_ge_mvmdio)
+		platform_device_register(orion_ge_mvmdio);
 	platform_device_register(orion_ge);
 }
 
@@ -258,8 +261,6 @@ struct mv643xx_eth_shared_platform_data orion_ge00_shared_data;
 static struct resource orion_ge00_shared_resources[] = {
 	{
 		.name	= "ge00 base",
-	}, {
-		.name	= "ge00 err irq",
 	},
 };
 
@@ -271,6 +272,19 @@ static struct platform_device orion_ge00_shared = {
 	},
 };
 
+static struct resource orion_ge00_mvmdio_resources[] = {
+	{
+		.name	= "ge00 mvmdio base",
+	}, {
+		.name	= "ge00 mvmdio irr irq",
+	},
+};
+
+static struct platform_device orion_ge00_mvmdio = {
+	.name		= "orion-mdio",
+	.id		= 0,
+};
+
 static struct resource orion_ge00_resources[] = {
 	{
 		.name	= "ge00 irq",
@@ -295,26 +309,25 @@ void __init orion_ge00_init(struct mv643xx_eth_platform_data *eth_data,
 			    unsigned int tx_csum_limit)
 {
 	fill_resources(&orion_ge00_shared, orion_ge00_shared_resources,
-		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+	fill_resources(&orion_ge00_mvmdio, orion_ge00_mvmdio_resources,
+			mapbase + 0x2004, 0x84 - 1, irq_err);
 	orion_ge00_shared_data.tx_csum_limit = tx_csum_limit;
 	ge_complete(&orion_ge00_shared_data,
 		    orion_ge00_resources, irq, &orion_ge00_shared,
+		    &orion_ge00_mvmdio,
 		    eth_data, &orion_ge00);
 }
 
 /*****************************************************************************
  * GE01
  ****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge01_shared_data = {
-	.shared_smi	= &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge01_shared_data;
 
 static struct resource orion_ge01_shared_resources[] = {
 	{
 		.name	= "ge01 base",
-	}, {
-		.name	= "ge01 err irq",
-	},
+	}
 };
 
 static struct platform_device orion_ge01_shared = {
@@ -325,6 +338,19 @@ static struct platform_device orion_ge01_shared = {
 	},
 };
 
+static struct resource orion_ge01_mvmdio_resources[] = {
+	{
+		.name	= "ge01 mdio base",
+	}, {
+		.name	= "ge01 mdio err irq",
+	},
+};
+
+static struct platform_device orion_ge01_mvmdio = {
+	.name		= "orion-mdio",
+	.id		= 1,
+};
+
 static struct resource orion_ge01_resources[] = {
 	{
 		.name	= "ge01 irq",
@@ -349,26 +375,25 @@ void __init orion_ge01_init(struct mv643xx_eth_platform_data *eth_data,
 			    unsigned int tx_csum_limit)
 {
 	fill_resources(&orion_ge01_shared, orion_ge01_shared_resources,
-		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+	fill_resources(&orion_ge01_mvmdio, orion_ge01_mvmdio_resources,
+			mapbase + 0x2004, 0x84 - 1, irq_err);
 	orion_ge01_shared_data.tx_csum_limit = tx_csum_limit;
 	ge_complete(&orion_ge01_shared_data,
 		    orion_ge01_resources, irq, &orion_ge01_shared,
+		    NULL,
 		    eth_data, &orion_ge01);
 }
 
 /*****************************************************************************
  * GE10
  ****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge10_shared_data = {
-	.shared_smi	= &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge10_shared_data;
 
 static struct resource orion_ge10_shared_resources[] = {
 	{
 		.name	= "ge10 base",
-	}, {
-		.name	= "ge10 err irq",
-	},
+	}
 };
 
 static struct platform_device orion_ge10_shared = {
@@ -379,6 +404,19 @@ static struct platform_device orion_ge10_shared = {
 	},
 };
 
+static struct resource orion_ge10_mvmdio_resources[] = {
+	{
+		.name	= "ge10 mvmdio base",
+	}, {
+		.name	= "ge10 mvmdio err irq",
+	},
+};
+
+static struct platform_device orion_ge10_mvmdio = {
+	.name		= "orion-mdio",
+	.id		= 1,
+};
+
 static struct resource orion_ge10_resources[] = {
 	{
 		.name	= "ge10 irq",
@@ -403,23 +441,22 @@ void __init orion_ge10_init(struct mv643xx_eth_platform_data *eth_data,
 {
 	fill_resources(&orion_ge10_shared, orion_ge10_shared_resources,
 		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+	fill_resources(&orion_ge10_mvmdio, orion_ge10_mvmdio_resources,
+		       mapbase + 0x2004, 0x84 - 1, irq_err);
 	ge_complete(&orion_ge10_shared_data,
 		    orion_ge10_resources, irq, &orion_ge10_shared,
+		    &orion_ge10_mvmdio,
 		    eth_data, &orion_ge10);
 }
 
 /*****************************************************************************
  * GE11
  ****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge11_shared_data = {
-	.shared_smi	= &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge11_shared_data;
 
 static struct resource orion_ge11_shared_resources[] = {
 	{
 		.name	= "ge11 base",
-	}, {
-		.name	= "ge11 err irq",
 	},
 };
 
@@ -431,6 +468,19 @@ static struct platform_device orion_ge11_shared = {
 	},
 };
 
+static struct resource orion_ge11_mvmdio_resources[] = {
+	{
+		.name	= "ge11 mvmdio base",
+	}, {
+		.name	= "ge11 mvmdio err irq",
+	},
+};
+
+static struct platform_device orion_ge11_mvmdio = {
+	.name		= "orion-mdio",
+	.id		= 1,
+};
+
 static struct resource orion_ge11_resources[] = {
 	{
 		.name	= "ge11 irq",
@@ -454,9 +504,12 @@ void __init orion_ge11_init(struct mv643xx_eth_platform_data *eth_data,
 			    unsigned long irq_err)
 {
 	fill_resources(&orion_ge11_shared, orion_ge11_shared_resources,
-		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+	fill_resources(&orion_ge11_mvmdio, orion_ge11_mvmdio_resources,
+		       mapbase + 0x2004, 0x84 - 1, irq_err);
 	ge_complete(&orion_ge11_shared_data,
 		    orion_ge11_resources, irq, &orion_ge11_shared,
+		    NULL,
 		    eth_data, &orion_ge11);
 }
 
diff --git a/arch/powerpc/platforms/chrp/pegasos_eth.c b/arch/powerpc/platforms/chrp/pegasos_eth.c
index 039fc8e..a671508 100644
--- a/arch/powerpc/platforms/chrp/pegasos_eth.c
+++ b/arch/powerpc/platforms/chrp/pegasos_eth.c
@@ -47,6 +47,25 @@ static struct platform_device mv643xx_eth_shared_device = {
 	.resource	= mv643xx_eth_shared_resources,
 };
 
+/*
+ * The orion mdio driver only covers shared + 0x4 up to shared + 0x84 - 1
+ */
+static struct resource mv643xx_eth_mvmdio_resources[] = {
+	[0] = {
+		.name	= "ethernet mdio base",
+		.start	= 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x4,
+		.end	= 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x83,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct platform_device mv643xx_eth_mvmdio_device = {
+	.name		= "orion-mdio",
+	.id		= 0,
+	.num_resources	= ARRAY_SIZE(mv643xx_eth_mvmdio_resources),
+	.resource	= mv643xx_eth_shared_resources,
+};
+
 static struct resource mv643xx_eth_port1_resources[] = {
 	[0] = {
 		.name	= "eth port1 irq",
@@ -82,6 +101,7 @@ static struct platform_device eth_port1_device = {
 
 static struct platform_device *mv643xx_eth_pd_devs[] __initdata = {
 	&mv643xx_eth_shared_device,
+	&mv643xx_eth_mvmdio_device,
 	&eth_port1_device,
 };
 
diff --git a/arch/powerpc/sysdev/mv64x60_dev.c b/arch/powerpc/sysdev/mv64x60_dev.c
index 0f6af41..630cea3 100644
--- a/arch/powerpc/sysdev/mv64x60_dev.c
+++ b/arch/powerpc/sysdev/mv64x60_dev.c
@@ -214,15 +214,25 @@ static struct platform_device * __init mv64x60_eth_register_shared_pdev(
 						struct device_node *np, int id)
 {
 	struct platform_device *pdev;
-	struct resource r[1];
+	struct resource r[2];
 	int err;
 
 	err = of_address_to_resource(np, 0, &r[0]);
 	if (err)
 		return ERR_PTR(err);
 
+	/* register an orion mdio bus driver */
+	r[1].start = r[0].start + 0x4;
+	r[1].end = r[0].start + 0x84 - 1;
+	r[1].flags = IORESOURCE_MEM;
+
+	pdev = platform_device_register_simple("orion-mdio", id, &r[1], 1);
+	if (!pdev)
+		return pdev;
+
 	pdev = platform_device_register_simple(MV643XX_ETH_SHARED_NAME, id,
-					       r, 1);
+					       &r[0], 1);
+
 	return pdev;
 }
 
diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index edfba93..df06061 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -23,6 +23,7 @@ config MV643XX_ETH
 	depends on (MV64X60 || PPC32 || PLAT_ORION) && INET
 	select INET_LRO
 	select PHYLIB
+	select MVMDIO
 	---help---
 	  This driver supports the gigabit ethernet MACs in the
 	  Marvell Discovery PPC/MIPS chipset family (MV643XX) and
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index d1ecf4b..df04bee 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -69,14 +69,6 @@ static char mv643xx_eth_driver_version[] = "1.4";
  * Registers shared between all ports.
  */
 #define PHY_ADDR			0x0000
-#define SMI_REG				0x0004
-#define  SMI_BUSY			0x10000000
-#define  SMI_READ_VALID			0x08000000
-#define  SMI_OPCODE_READ		0x04000000
-#define  SMI_OPCODE_WRITE		0x00000000
-#define ERR_INT_CAUSE			0x0080
-#define  ERR_INT_SMI_DONE		0x00000010
-#define ERR_INT_MASK			0x0084
 #define WINDOW_BASE(w)			(0x0200 + ((w) << 3))
 #define WINDOW_SIZE(w)			(0x0204 + ((w) << 3))
 #define WINDOW_REMAP_HIGH(w)		(0x0280 + ((w) << 2))
@@ -266,25 +258,6 @@ struct mv643xx_eth_shared_private {
 	void __iomem *base;
 
 	/*
-	 * Points at the right SMI instance to use.
-	 */
-	struct mv643xx_eth_shared_private *smi;
-
-	/*
-	 * Provides access to local SMI interface.
-	 */
-	struct mii_bus *smi_bus;
-
-	/*
-	 * If we have access to the error interrupt pin (which is
-	 * somewhat misnamed as it not only reflects internal errors
-	 * but also reflects SMI completion), use that to wait for
-	 * SMI access completion instead of polling the SMI busy bit.
-	 */
-	int err_interrupt;
-	wait_queue_head_t smi_busy_wait;
-
-	/*
 	 * Per-port MBUS window access register value.
 	 */
 	u32 win_protect;
@@ -1122,97 +1095,6 @@ out_write:
 	wrlp(mp, PORT_SERIAL_CONTROL, pscr);
 }
 
-static irqreturn_t mv643xx_eth_err_irq(int irq, void *dev_id)
-{
-	struct mv643xx_eth_shared_private *msp = dev_id;
-
-	if (readl(msp->base + ERR_INT_CAUSE) & ERR_INT_SMI_DONE) {
-		writel(~ERR_INT_SMI_DONE, msp->base + ERR_INT_CAUSE);
-		wake_up(&msp->smi_busy_wait);
-		return IRQ_HANDLED;
-	}
-
-	return IRQ_NONE;
-}
-
-static int smi_is_done(struct mv643xx_eth_shared_private *msp)
-{
-	return !(readl(msp->base + SMI_REG) & SMI_BUSY);
-}
-
-static int smi_wait_ready(struct mv643xx_eth_shared_private *msp)
-{
-	if (msp->err_interrupt == NO_IRQ) {
-		int i;
-
-		for (i = 0; !smi_is_done(msp); i++) {
-			if (i == 10)
-				return -ETIMEDOUT;
-			msleep(10);
-		}
-
-		return 0;
-	}
-
-	if (!smi_is_done(msp)) {
-		wait_event_timeout(msp->smi_busy_wait, smi_is_done(msp),
-				   msecs_to_jiffies(100));
-		if (!smi_is_done(msp))
-			return -ETIMEDOUT;
-	}
-
-	return 0;
-}
-
-static int smi_bus_read(struct mii_bus *bus, int addr, int reg)
-{
-	struct mv643xx_eth_shared_private *msp = bus->priv;
-	void __iomem *smi_reg = msp->base + SMI_REG;
-	int ret;
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	writel(SMI_OPCODE_READ | (reg << 21) | (addr << 16), smi_reg);
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	ret = readl(smi_reg);
-	if (!(ret & SMI_READ_VALID)) {
-		pr_warn("SMI bus read not valid\n");
-		return -ENODEV;
-	}
-
-	return ret & 0xffff;
-}
-
-static int smi_bus_write(struct mii_bus *bus, int addr, int reg, u16 val)
-{
-	struct mv643xx_eth_shared_private *msp = bus->priv;
-	void __iomem *smi_reg = msp->base + SMI_REG;
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	writel(SMI_OPCODE_WRITE | (reg << 21) |
-		(addr << 16) | (val & 0xffff), smi_reg);
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	return 0;
-}
-
-
 /* statistics ***************************************************************/
 static struct net_device_stats *mv643xx_eth_get_stats(struct net_device *dev)
 {
@@ -2688,47 +2570,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 		goto out_free;
 
 	/*
-	 * Set up and register SMI bus.
-	 */
-	if (pd == NULL || pd->shared_smi == NULL) {
-		msp->smi_bus = mdiobus_alloc();
-		if (msp->smi_bus == NULL)
-			goto out_unmap;
-
-		msp->smi_bus->priv = msp;
-		msp->smi_bus->name = "mv643xx_eth smi";
-		msp->smi_bus->read = smi_bus_read;
-		msp->smi_bus->write = smi_bus_write,
-		snprintf(msp->smi_bus->id, MII_BUS_ID_SIZE, "%s-%d",
-			pdev->name, pdev->id);
-		msp->smi_bus->parent = &pdev->dev;
-		msp->smi_bus->phy_mask = 0xffffffff;
-		if (mdiobus_register(msp->smi_bus) < 0)
-			goto out_free_mii_bus;
-		msp->smi = msp;
-	} else {
-		msp->smi = platform_get_drvdata(pd->shared_smi);
-	}
-
-	msp->err_interrupt = NO_IRQ;
-	init_waitqueue_head(&msp->smi_busy_wait);
-
-	/*
-	 * Check whether the error interrupt is hooked up.
-	 */
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (res != NULL) {
-		int err;
-
-		err = request_irq(res->start, mv643xx_eth_err_irq,
-				  IRQF_SHARED, "mv643xx_eth", msp);
-		if (!err) {
-			writel(ERR_INT_SMI_DONE, msp->base + ERR_INT_MASK);
-			msp->err_interrupt = res->start;
-		}
-	}
-
-	/*
 	 * (Re-)program MBUS remapping windows if we are asked to.
 	 */
 	dram = mv_mbus_dram_info();
@@ -2743,10 +2584,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 
 	return 0;
 
-out_free_mii_bus:
-	mdiobus_free(msp->smi_bus);
-out_unmap:
-	iounmap(msp->base);
 out_free:
 	kfree(msp);
 out:
@@ -2756,14 +2593,7 @@ out:
 static int mv643xx_eth_shared_remove(struct platform_device *pdev)
 {
 	struct mv643xx_eth_shared_private *msp = platform_get_drvdata(pdev);
-	struct mv643xx_eth_shared_platform_data *pd = pdev->dev.platform_data;
 
-	if (pd == NULL || pd->shared_smi == NULL) {
-		mdiobus_unregister(msp->smi_bus);
-		mdiobus_free(msp->smi_bus);
-	}
-	if (msp->err_interrupt != NO_IRQ)
-		free_irq(msp->err_interrupt, msp);
 	iounmap(msp->base);
 	kfree(msp);
 
@@ -2829,11 +2659,11 @@ static void set_params(struct mv643xx_eth_private *mp,
 static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
 				   int phy_addr)
 {
-	struct mii_bus *bus = mp->shared->smi->smi_bus;
 	struct phy_device *phydev;
 	int start;
 	int num;
 	int i;
+	char phy_id[MII_BUS_ID_SIZE + 3];
 
 	if (phy_addr == MV643XX_ETH_PHY_ADDR_DEFAULT) {
 		start = phy_addr_get(mp) & 0x1f;
@@ -2843,17 +2673,19 @@ static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
 		num = 1;
 	}
 
+	/* Attempt to connect to the PHY using orion-mdio */
 	phydev = NULL;
 	for (i = 0; i < num; i++) {
 		int addr = (start + i) & 0x1f;
 
-		if (bus->phy_map[addr] == NULL)
-			mdiobus_scan(bus, addr);
+		snprintf(phy_id, sizeof(phy_id), PHY_ID_FMT,
+				"orion-mdio-mii", addr);
 
-		if (phydev == NULL) {
-			phydev = bus->phy_map[addr];
-			if (phydev != NULL)
-				phy_addr_set(mp, addr);
+		phydev = phy_connect(mp->dev, phy_id, NULL,
+				PHY_INTERFACE_MODE_GMII);
+		if (!IS_ERR(phydev)) {
+			phy_addr_set(mp, addr);
+			break;
 		}
 	}
 
diff --git a/include/linux/mv643xx_eth.h b/include/linux/mv643xx_eth.h
index 49258e0..141d395 100644
--- a/include/linux/mv643xx_eth.h
+++ b/include/linux/mv643xx_eth.h
@@ -19,7 +19,6 @@
 
 struct mv643xx_eth_shared_platform_data {
 	struct mbus_dram_target_info	*dram;
-	struct platform_device	*shared_smi;
 	/*
 	 * Max packet size for Tx IP/Layer 4 checksum, when set to 0, default
 	 * limit of 9KiB will be used.
-- 
1.7.10.4

Re: [PATCH 4/4 v2] mv643xx_eth: convert to use the Marvell Orion MDIO driver

[Florian Fainelli]

Le 03/14/13 19:08, Florian Fainelli a écrit :
> This patch converts the Marvell MV643XX ethernet driver to use the
> Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
> registering the Marvell MV643XX ethernet driver are also updated to
> register a Marvell Orion MDIO driver. This driver voluntarily overlaps
> with the Marvell Ethernet shared registers because it will use a subset
> of this shared register (shared_base + 0x4 - shared_base + 0x84). The
> Ethernet driver is also updated to look up for a PHY device using the
> Orion MDIO bus driver.

Thanks to the help of Andrew Lunn, there is at least two known issues 
with this patch version:

- we need to move up the mvmdio line in 
drivers/net/ethernet/marvell/Makefile to make sure that configs having 
both mvmdio and mv643xx_eth built-in get the probing order right
- the bus name used by mv643xx_eth is not the right now (orion-mdio.0 vs 
expected orion-mdio) so the PHY device will not be found during 
phy_connect()

I will fix these two issues in the next version of the patchset.
--
Florian

Re: [PATCH 4/4 v2] mv643xx_eth: convert to use the Marvell Orion MDIO driver

[Thomas Petazzoni]

Dear Florian Fainelli,

On Fri, 15 Mar 2013 12:07:12 +0100, Florian Fainelli wrote:

> Thanks to the help of Andrew Lunn, there is at least two known issues 
> with this patch version:
> 
> - we need to move up the mvmdio line in 
> drivers/net/ethernet/marvell/Makefile to make sure that configs having 
> both mvmdio and mv643xx_eth built-in get the probing order right

I don't think it's the right way of fixing the problem. If there is no
dependency on the two devices through the device model (i.e they don't
have a parent->child relationship), then the mv643xx_eth driver should
probably return -EPROBE_DEFER when it can't find its PHY so that its
->probe() operation gets called once again by the kernel when other
drivers (including mvmdio) have been probed.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 1/4 v2] net: mvmdio: allow platform device style registration

[Florian Fainelli]

This patch changes the mvmdio driver not to use device tree
helper functions such as of_mdiobus_register() and of_iomap() so we can
instantiate this driver using a classic platform_device approach. Use
the device manager helper to ioremap() the base register cookie so we
get automatic freeing upon error and removal. This change is harmless
for Device Tree platforms because they will get the driver be registered
the same way as it was before.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
Changes since v1:
- use device managed helpers to fix an ioremap leak in remove function
- remove the use of Device Tree specific helpers to allow platform-style
  registration

 drivers/net/ethernet/marvell/mvmdio.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 77b7c80..8a182f1 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -24,10 +24,9 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/phy.h>
-#include <linux/of_address.h>
-#include <linux/of_mdio.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
+#include <linux/io.h>
 
 #define MVMDIO_SMI_DATA_SHIFT              0
 #define MVMDIO_SMI_PHY_ADDR_SHIFT          16
@@ -143,11 +142,17 @@ static int orion_mdio_reset(struct mii_bus *bus)
 
 static int orion_mdio_probe(struct platform_device *pdev)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct resource *r;
 	struct mii_bus *bus;
 	struct orion_mdio_dev *dev;
 	int i, ret;
 
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r) {
+		dev_err(&pdev->dev, "No SMI register address given\n");
+		return -ENODEV;
+	}
+
 	bus = mdiobus_alloc_size(sizeof(struct orion_mdio_dev));
 	if (!bus) {
 		dev_err(&pdev->dev, "Cannot allocate MDIO bus\n");
@@ -172,9 +177,9 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		bus->irq[i] = PHY_POLL;
 
 	dev = bus->priv;
-	dev->smireg = of_iomap(pdev->dev.of_node, 0);
+	dev->smireg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
 	if (!dev->smireg) {
-		dev_err(&pdev->dev, "No SMI register address given in DT\n");
+		dev_err(&pdev->dev, "Unable to remap SMI register\n");
 		kfree(bus->irq);
 		mdiobus_free(bus);
 		return -ENODEV;
@@ -182,10 +187,9 @@ static int orion_mdio_probe(struct platform_device *pdev)
 
 	mutex_init(&dev->lock);
 
-	ret = of_mdiobus_register(bus, np);
+	ret = mdiobus_register(bus);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
-		iounmap(dev->smireg);
 		kfree(bus->irq);
 		mdiobus_free(bus);
 		return ret;
-- 
1.7.10.4

Re: [PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver

[David Miller]

From: Florian Fainelli <florian@openwrt.org>
Date: Thu, 14 Mar 2013 19:08:31 +0100

> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
> instead of rolling its own implementation. As a result, all users of this
> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
> The mvmdio driver is also updated to support an interrupt line which reports
> SMI error/completion, and to allow traditionnal platform device registration
> instead of just device tree.
> 
> David, I think it makes sense for you to merge all of this, since we do
> not want the architecture files to be desynchronized from the mv643xx_eth to
> avoid runtime breakage. The potential for merge conflicts should be very small.

All applied to net-next, thanks.

Re: [PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver

[Florian Fainelli]

Le 03/15/13 13:53, David Miller a écrit :
> From: Florian Fainelli <florian@openwrt.org>
> Date: Thu, 14 Mar 2013 19:08:31 +0100
>
>> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
>> instead of rolling its own implementation. As a result, all users of this
>> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
>> The mvmdio driver is also updated to support an interrupt line which reports
>> SMI error/completion, and to allow traditionnal platform device registration
>> instead of just device tree.
>>
>> David, I think it makes sense for you to merge all of this, since we do
>> not want the architecture files to be desynchronized from the mv643xx_eth to
>> avoid runtime breakage. The potential for merge conflicts should be very small.
>
> All applied to net-next, thanks.
>

Oh woah that was fast, maybe too fast, I will submit a follow-up patch 
for patch 4 to address the issues I mentionned earlier.
--
Florian

Re: [PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver

[David Miller]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: Text/Plain; charset=iso-8859-7, Size: 1731 bytes --]

From: David Miller <davem@davemloft.net>
Date: Fri, 15 Mar 2013 08:53:21 -0400 (EDT)

> From: Florian Fainelli <florian@openwrt.org>
> Date: Thu, 14 Mar 2013 19:08:31 +0100
> 
>> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
>> instead of rolling its own implementation. As a result, all users of this
>> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
>> The mvmdio driver is also updated to support an interrupt line which reports
>> SMI error/completion, and to allow traditionnal platform device registration
>> instead of just device tree.
>> 
>> David, I think it makes sense for you to merge all of this, since we do
>> not want the architecture files to be desynchronized from the mv643xx_eth to
>> avoid runtime breakage. The potential for merge conflicts should be very small.
> 
> All applied to net-next, thanks.

Actually, reverted.  Please send me code which actually compiles:

drivers/net/ethernet/marvell/mvmdio.c: In function ¡orion_mdio_wait_ready¢:
drivers/net/ethernet/marvell/mvmdio.c:70:28: error: ¡NO_IRQ¢ undeclared (first use in this function)
drivers/net/ethernet/marvell/mvmdio.c:70:28: note: each undeclared identifier is reported only once for each function it appears in
drivers/net/ethernet/marvell/mvmdio.c: In function ¡orion_mdio_probe¢:
drivers/net/ethernet/marvell/mvmdio.c:242:24: error: ¡NO_IRQ¢ undeclared (first use in this function)
make[4]: *** [drivers/net/ethernet/marvell/mvmdio.o] Error 1

And don't use Kconfig dependencies to work around this, fix it properly.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver

[Florian Fainelli]

Le 03/15/13 13:55, David Miller a écrit :
> From: David Miller <davem@davemloft.net>
> Date: Fri, 15 Mar 2013 08:53:21 -0400 (EDT)
>
>> From: Florian Fainelli <florian@openwrt.org>
>> Date: Thu, 14 Mar 2013 19:08:31 +0100
>>
>>> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
>>> instead of rolling its own implementation. As a result, all users of this
>>> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
>>> The mvmdio driver is also updated to support an interrupt line which reports
>>> SMI error/completion, and to allow traditionnal platform device registration
>>> instead of just device tree.
>>>
>>> David, I think it makes sense for you to merge all of this, since we do
>>> not want the architecture files to be desynchronized from the mv643xx_eth to
>>> avoid runtime breakage. The potential for merge conflicts should be very small.
>>
>> All applied to net-next, thanks.
>
> Actually, reverted.  Please send me code which actually compiles:
>
> drivers/net/ethernet/marvell/mvmdio.c: In function ‘orion_mdio_wait_ready’:
> drivers/net/ethernet/marvell/mvmdio.c:70:28: error: ‘NO_IRQ’ undeclared (first use in this function)
> drivers/net/ethernet/marvell/mvmdio.c:70:28: note: each undeclared identifier is reported only once for each function it appears in
> drivers/net/ethernet/marvell/mvmdio.c: In function ‘orion_mdio_probe’:
> drivers/net/ethernet/marvell/mvmdio.c:242:24: error: ‘NO_IRQ’ undeclared (first use in this function)
> make[4]: *** [drivers/net/ethernet/marvell/mvmdio.o] Error 1
>
> And don't use Kconfig dependencies to work around this, fix it properly.

Is there any platform out there for which we do not have a NO_IRQ 
definition by now? If so, what is it?
--
Florian

Re: [PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver

[David Miller]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: Text/Plain; charset=utf-8, Size: 2455 bytes --]

From: Florian Fainelli <florian@openwrt.org>
Date: Fri, 15 Mar 2013 13:53:10 +0100

> Le 03/15/13 13:55, David Miller a écrit :
>> From: David Miller <davem@davemloft.net>
>> Date: Fri, 15 Mar 2013 08:53:21 -0400 (EDT)
>>
>>> From: Florian Fainelli <florian@openwrt.org>
>>> Date: Thu, 14 Mar 2013 19:08:31 +0100
>>>
>>>> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus
>>>> driver
>>>> instead of rolling its own implementation. As a result, all users of
>>>> this
>>>> mv643xx_eth driver are converted to register an "orion-mdio"
>>>> platform_device.
>>>> The mvmdio driver is also updated to support an interrupt line which
>>>> reports
>>>> SMI error/completion, and to allow traditionnal platform device
>>>> registration
>>>> instead of just device tree.
>>>>
>>>> David, I think it makes sense for you to merge all of this, since we
>>>> do
>>>> not want the architecture files to be desynchronized from the
>>>> mv643xx_eth to
>>>> avoid runtime breakage. The potential for merge conflicts should be
>>>> very small.
>>>
>>> All applied to net-next, thanks.
>>
>> Actually, reverted.  Please send me code which actually compiles:
>>
>> drivers/net/ethernet/marvell/mvmdio.c: In function
>> ‘orion_mdio_wait_ready’:
>> drivers/net/ethernet/marvell/mvmdio.c:70:28: error: ‘NO_IRQ’
>> undeclared (first use in this function)
>> drivers/net/ethernet/marvell/mvmdio.c:70:28: note: each undeclared
>> identifier is reported only once for each function it appears in
>> drivers/net/ethernet/marvell/mvmdio.c: In function ‘orion_mdio_probe’:
>> drivers/net/ethernet/marvell/mvmdio.c:242:24: error: ‘NO_IRQ’
>> undeclared (first use in this function)
>> make[4]: *** [drivers/net/ethernet/marvell/mvmdio.o] Error 1
>>
>> And don't use Kconfig dependencies to work around this, fix it
>> properly.
> 
> Is there any platform out there for which we do not have a NO_IRQ
> definition by now? If so, what is it?

Obviously if x86_64 doesn't even build your changes, that is one such
platform.  Also, is grep not working on your computer?

Platforms are absolutely no required to have this define, zero is the
only valid "no IRQ" which is portable in any way.

This is an old and tired topic, portable code does not use NO_IRQ, and
that's simply the end of it.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver

[Florian Fainelli]

Le 03/15/13 14:05, David Miller a écrit :
> From: Florian Fainelli <florian@openwrt.org>
> Date: Fri, 15 Mar 2013 13:53:10 +0100
>
>> Le 03/15/13 13:55, David Miller a écrit :
>>> From: David Miller <davem@davemloft.net>
>>> Date: Fri, 15 Mar 2013 08:53:21 -0400 (EDT)
>>>
>>>> From: Florian Fainelli <florian@openwrt.org>
>>>> Date: Thu, 14 Mar 2013 19:08:31 +0100
>>>>
>>>>> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus
>>>>> driver
>>>>> instead of rolling its own implementation. As a result, all users of
>>>>> this
>>>>> mv643xx_eth driver are converted to register an "orion-mdio"
>>>>> platform_device.
>>>>> The mvmdio driver is also updated to support an interrupt line which
>>>>> reports
>>>>> SMI error/completion, and to allow traditionnal platform device
>>>>> registration
>>>>> instead of just device tree.
>>>>>
>>>>> David, I think it makes sense for you to merge all of this, since we
>>>>> do
>>>>> not want the architecture files to be desynchronized from the
>>>>> mv643xx_eth to
>>>>> avoid runtime breakage. The potential for merge conflicts should be
>>>>> very small.
>>>>
>>>> All applied to net-next, thanks.
>>>
>>> Actually, reverted.  Please send me code which actually compiles:
>>>
>>> drivers/net/ethernet/marvell/mvmdio.c: In function
>>> ‘orion_mdio_wait_ready’:
>>> drivers/net/ethernet/marvell/mvmdio.c:70:28: error: ‘NO_IRQ’
>>> undeclared (first use in this function)
>>> drivers/net/ethernet/marvell/mvmdio.c:70:28: note: each undeclared
>>> identifier is reported only once for each function it appears in
>>> drivers/net/ethernet/marvell/mvmdio.c: In function ‘orion_mdio_probe’:
>>> drivers/net/ethernet/marvell/mvmdio.c:242:24: error: ‘NO_IRQ’
>>> undeclared (first use in this function)
>>> make[4]: *** [drivers/net/ethernet/marvell/mvmdio.o] Error 1
>>>
>>> And don't use Kconfig dependencies to work around this, fix it
>>> properly.
>>
>> Is there any platform out there for which we do not have a NO_IRQ
>> definition by now? If so, what is it?
>
> Obviously if x86_64 doesn't even build your changes, that is one such
> platform.  Also, is grep not working on your computer?

I built tested on PowerPC and ARM which are the platforms actually 
*using* these drivers and forgot that you build for x86_64.

>
> Platforms are absolutely no required to have this define, zero is the
> only valid "no IRQ" which is portable in any way.
>
> This is an old and tired topic, portable code does not use NO_IRQ, and
> that's simply the end of it.

I changed not to rely on NO_IRQ anymore. Thanks!
--
Florian

[PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver

[Florian Fainelli]

Hi all,

This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
instead of rolling its own implementation. As a result, all users of this
mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
The mvmdio driver is also updated to support an interrupt line which reports
SMI error/completion, and to allow traditionnal platform device registration
instead of just device tree.

David, I think it makes sense for you to merge all of this, since we do
not want the architecture files to be desynchronized from the mv643xx_eth to
avoid runtime breakage. The potential for merge conflicts should be very small.

Florian Fainelli (4):
  net: mvmdio: allow platform device style registration
  net: mvmdio: rename base register cookie from smireg to regs
  net: mvmdio: enhance driver to support SMI error/done interrupts
  mv643xx_eth: convert to use the Marvell Orion MDIO driver

 .../devicetree/bindings/net/marvell-orion-mdio.txt |    3 +
 arch/arm/plat-orion/common.c                       |   54 +++---
 arch/powerpc/platforms/chrp/pegasos_eth.c          |   20 ++
 arch/powerpc/sysdev/mv64x60_dev.c                  |   16 +-
 drivers/net/ethernet/marvell/Kconfig               |    5 +-
 drivers/net/ethernet/marvell/Makefile              |    2 +-
 drivers/net/ethernet/marvell/mv643xx_eth.c         |  195 ++------------------
 drivers/net/ethernet/marvell/mvmdio.c              |  130 ++++++++++---
 include/linux/mv643xx_eth.h                        |    1 -
 9 files changed, 187 insertions(+), 239 deletions(-)

-- 
1.7.10.4

[PATCH 1/4 v3] net: mvmdio: allow platform device style registration

[Florian Fainelli]

This patch changes the mvmdio driver not to use device tree
helper functions such as of_mdiobus_register() and of_iomap() so we can
instantiate this driver using a classic platform_device approach. Use
the device manager helper to ioremap() the base register cookie so we
get automatic freeing upon error and removal. This change is harmless
for Device Tree platforms because they will get the driver be registered
the same way as it was before.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
Changes since v2:
- use of_mdiobus_register if device tree node is available to ensure that
  all child PHY nodes get also registered (fixes PHY probing on DT-only)

Changes since v1:
- use device managed helpers to fix an ioremap leak in remove function
- remove the use of Device Tree specific helpers to allow platform-style
  registration

 drivers/net/ethernet/marvell/mvmdio.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 77b7c80..bbc5fde 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -24,10 +24,10 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/phy.h>
-#include <linux/of_address.h>
-#include <linux/of_mdio.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of_mdio.h>
 
 #define MVMDIO_SMI_DATA_SHIFT              0
 #define MVMDIO_SMI_PHY_ADDR_SHIFT          16
@@ -143,11 +143,17 @@ static int orion_mdio_reset(struct mii_bus *bus)
 
 static int orion_mdio_probe(struct platform_device *pdev)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct resource *r;
 	struct mii_bus *bus;
 	struct orion_mdio_dev *dev;
 	int i, ret;
 
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r) {
+		dev_err(&pdev->dev, "No SMI register address given\n");
+		return -ENODEV;
+	}
+
 	bus = mdiobus_alloc_size(sizeof(struct orion_mdio_dev));
 	if (!bus) {
 		dev_err(&pdev->dev, "Cannot allocate MDIO bus\n");
@@ -172,9 +178,9 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		bus->irq[i] = PHY_POLL;
 
 	dev = bus->priv;
-	dev->smireg = of_iomap(pdev->dev.of_node, 0);
+	dev->smireg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
 	if (!dev->smireg) {
-		dev_err(&pdev->dev, "No SMI register address given in DT\n");
+		dev_err(&pdev->dev, "Unable to remap SMI register\n");
 		kfree(bus->irq);
 		mdiobus_free(bus);
 		return -ENODEV;
@@ -182,10 +188,12 @@ static int orion_mdio_probe(struct platform_device *pdev)
 
 	mutex_init(&dev->lock);
 
-	ret = of_mdiobus_register(bus, np);
+	if (pdev->dev.of_node)
+		ret = of_mdiobus_register(bus, pdev->dev.of_node);
+	else
+		ret = mdiobus_register(bus);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
-		iounmap(dev->smireg);
 		kfree(bus->irq);
 		mdiobus_free(bus);
 		return ret;
-- 
1.7.10.4

[PATCH 2/4 v3] net: mvmdio: rename base register cookie from smireg to regs

[Florian Fainelli]

This patch renames the base register cookie in the mvmdio drive from
"smireg" to "regs" since a subsequent patch is going to use an ioremap()
cookie whose size is larger than a single register of 4 bytes. No
functionnal code change introduced.

Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
Changes since v2:
- rebased against latest version

Changes since v1:
- added Thomas Acked-by tag

 drivers/net/ethernet/marvell/mvmdio.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index bbc5fde..3e2711d 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -39,7 +39,7 @@
 
 struct orion_mdio_dev {
 	struct mutex lock;
-	void __iomem *smireg;
+	void __iomem *regs;
 };
 
 /* Wait for the SMI unit to be ready for another operation
@@ -52,7 +52,7 @@ static int orion_mdio_wait_ready(struct mii_bus *bus)
 
 	count = 0;
 	while (1) {
-		val = readl(dev->smireg);
+		val = readl(dev->regs);
 		if (!(val & MVMDIO_SMI_BUSY))
 			break;
 
@@ -87,12 +87,12 @@ static int orion_mdio_read(struct mii_bus *bus, int mii_id,
 	writel(((mii_id << MVMDIO_SMI_PHY_ADDR_SHIFT) |
 		(regnum << MVMDIO_SMI_PHY_REG_SHIFT)  |
 		MVMDIO_SMI_READ_OPERATION),
-	       dev->smireg);
+	       dev->regs);
 
 	/* Wait for the value to become available */
 	count = 0;
 	while (1) {
-		val = readl(dev->smireg);
+		val = readl(dev->regs);
 		if (val & MVMDIO_SMI_READ_VALID)
 			break;
 
@@ -129,7 +129,7 @@ static int orion_mdio_write(struct mii_bus *bus, int mii_id,
 		(regnum << MVMDIO_SMI_PHY_REG_SHIFT)  |
 		MVMDIO_SMI_WRITE_OPERATION            |
 		(value << MVMDIO_SMI_DATA_SHIFT)),
-	       dev->smireg);
+	       dev->regs);
 
 	mutex_unlock(&dev->lock);
 
@@ -178,8 +178,8 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		bus->irq[i] = PHY_POLL;
 
 	dev = bus->priv;
-	dev->smireg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
-	if (!dev->smireg) {
+	dev->regs = devm_ioremap(&pdev->dev, r->start, resource_size(r));
+	if (!dev->regs) {
 		dev_err(&pdev->dev, "Unable to remap SMI register\n");
 		kfree(bus->irq);
 		mdiobus_free(bus);
-- 
1.7.10.4

[PATCH 3/4 v3] net: mvmdio: enhance driver to support SMI error/done interrupts

[Florian Fainelli]

This patch enhances the "mvmdio" to support a SMI error/done interrupt
line which can be used along with a wait queue instead of doing
busy-waiting on the registers. This is a feature which is available in
the mv643xx_eth SMI code and thus reduces again the gap between the two.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
Changes since v2:
- remove NO_IRQ usage which is not portable, check for irq >= 0
  instead
- factor out mdiobus cleaning under the out_mdio label in probe()

Chances since v1:
- always use orion_smi_is_done() helper
- make interrupt/non-interrupt code path entirely independant

 .../devicetree/bindings/net/marvell-orion-mdio.txt |    3 +
 drivers/net/ethernet/marvell/mvmdio.c              |   98 ++++++++++++++++----
 2 files changed, 83 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
index 34e7aaf..052b5f2 100644
--- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
+++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
@@ -9,6 +9,9 @@ Required properties:
 - compatible: "marvell,orion-mdio"
 - reg: address and length of the SMI register
 
+Optional properties:
+- interrupts: interrupt line number for the SMI error/done interrupt
+
 The child nodes of the MDIO driver are the individual PHY devices
 connected to this MDIO bus. They must have a "reg" property given the
 PHY address on the MDIO bus.
diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 3e2711d..3472574 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -24,10 +24,13 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/phy.h>
+#include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/of_mdio.h>
+#include <linux/sched.h>
+#include <linux/wait.h>
 
 #define MVMDIO_SMI_DATA_SHIFT              0
 #define MVMDIO_SMI_PHY_ADDR_SHIFT          16
@@ -36,33 +39,58 @@
 #define MVMDIO_SMI_WRITE_OPERATION         0
 #define MVMDIO_SMI_READ_VALID              BIT(27)
 #define MVMDIO_SMI_BUSY                    BIT(28)
+#define MVMDIO_ERR_INT_CAUSE		   0x007C
+#define  MVMDIO_ERR_INT_SMI_DONE	   0x00000010
+#define MVMDIO_ERR_INT_MASK		   0x0080
 
 struct orion_mdio_dev {
 	struct mutex lock;
 	void __iomem *regs;
+	/*
+	 * If we have access to the error interrupt pin (which is
+	 * somewhat misnamed as it not only reflects internal errors
+	 * but also reflects SMI completion), use that to wait for
+	 * SMI access completion instead of polling the SMI busy bit.
+	 */
+	int err_interrupt;
+	wait_queue_head_t smi_busy_wait;
 };
 
+static int orion_mdio_smi_is_done(struct orion_mdio_dev *dev)
+{
+	return !(readl(dev->regs) & MVMDIO_SMI_BUSY);
+}
+
 /* Wait for the SMI unit to be ready for another operation
  */
 static int orion_mdio_wait_ready(struct mii_bus *bus)
 {
 	struct orion_mdio_dev *dev = bus->priv;
 	int count;
-	u32 val;
 
-	count = 0;
-	while (1) {
-		val = readl(dev->regs);
-		if (!(val & MVMDIO_SMI_BUSY))
-			break;
+	if (dev->err_interrupt <= 0) {
+		count = 0;
+		while (1) {
+			if (orion_mdio_smi_is_done(dev))
+				break;
 
-		if (count > 100) {
-			dev_err(bus->parent, "Timeout: SMI busy for too long\n");
-			return -ETIMEDOUT;
-		}
+			if (count > 100) {
+				dev_err(bus->parent,
+					"Timeout: SMI busy for too long\n");
+				return -ETIMEDOUT;
+			}
 
-		udelay(10);
-		count++;
+			udelay(10);
+			count++;
+		}
+	} else {
+		if (!orion_mdio_smi_is_done(dev)) {
+			wait_event_timeout(dev->smi_busy_wait,
+				orion_mdio_smi_is_done(dev),
+				msecs_to_jiffies(100));
+			if (!orion_mdio_smi_is_done(dev))
+				return -ETIMEDOUT;
+		}
 	}
 
 	return 0;
@@ -141,6 +169,21 @@ static int orion_mdio_reset(struct mii_bus *bus)
 	return 0;
 }
 
+static irqreturn_t orion_mdio_err_irq(int irq, void *dev_id)
+{
+	struct orion_mdio_dev *dev = dev_id;
+
+	if (readl(dev->regs + MVMDIO_ERR_INT_CAUSE) &
+			MVMDIO_ERR_INT_SMI_DONE) {
+		writel(~MVMDIO_ERR_INT_SMI_DONE,
+				dev->regs + MVMDIO_ERR_INT_CAUSE);
+		wake_up(&dev->smi_busy_wait);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
 static int orion_mdio_probe(struct platform_device *pdev)
 {
 	struct resource *r;
@@ -181,9 +224,22 @@ static int orion_mdio_probe(struct platform_device *pdev)
 	dev->regs = devm_ioremap(&pdev->dev, r->start, resource_size(r));
 	if (!dev->regs) {
 		dev_err(&pdev->dev, "Unable to remap SMI register\n");
-		kfree(bus->irq);
-		mdiobus_free(bus);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_mdio;
+	}
+
+	init_waitqueue_head(&dev->smi_busy_wait);
+
+	dev->err_interrupt = platform_get_irq(pdev, 0);
+	if (dev->err_interrupt != -ENXIO) {
+		ret = devm_request_irq(&pdev->dev, dev->err_interrupt,
+					orion_mdio_err_irq,
+					IRQF_SHARED, pdev->name, dev);
+		if (ret)
+			goto out_mdio;
+
+		writel(MVMDIO_ERR_INT_SMI_DONE,
+			dev->regs + MVMDIO_ERR_INT_MASK);
 	}
 
 	mutex_init(&dev->lock);
@@ -194,19 +250,25 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		ret = mdiobus_register(bus);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
-		kfree(bus->irq);
-		mdiobus_free(bus);
-		return ret;
+		goto out_mdio;
 	}
 
 	platform_set_drvdata(pdev, bus);
 
 	return 0;
+
+out_mdio:
+	kfree(bus->irq);
+	mdiobus_free(bus);
+	return ret;
 }
 
 static int orion_mdio_remove(struct platform_device *pdev)
 {
 	struct mii_bus *bus = platform_get_drvdata(pdev);
+	struct orion_mdio_dev *dev = bus->priv;
+
+	writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
 	mdiobus_unregister(bus);
 	kfree(bus->irq);
 	mdiobus_free(bus);
-- 
1.7.10.4

[PATCH 4/4 v3] mv643xx_eth: convert to use the Marvell Orion MDIO driver

[Florian Fainelli]

This patch converts the Marvell MV643XX ethernet driver to use the
Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
registering the Marvell MV643XX ethernet driver are also updated to
register a Marvell Orion MDIO driver. This driver voluntarily overlaps
with the Marvell Ethernet shared registers because it will use a subset
of this shared register (shared_base + 0x4 to shared_base + 0x84). The
Ethernet driver is also updated to look up for a PHY device using the
Orion MDIO bus driver.

For ARM and PowerPC we register a single instance of the "mvmdio" driver
in the system like it used to be done with the use of the "shared_smi"
platform_data cookie on ARM.

Note that it is safe to register the mvmdio driver only for the "ge00"
instance of the driver because this "ge00" interface is guaranteed to
always be explicitely registered by consumers of
arch/arm/plat-orion/common.c and other instances (ge01, ge10 and ge11)
were all pointing their shared_smi to ge00. For PowerPC the in-tree
Device Tree Source files mention only one MV643XX ethernet MAC instance
so the MDIO bus driver is registered only when id == 0.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
Changes since v2:
- added missing adjust_link callback to phy_connect
- removed superfluous phy_attach() call
- modified PowerPC code to register a single orion-mdio instance
- modified arm code to register a single orion-mdio instance for
  all GE* interfaces

Changes since v1:
- removed one too many mdio bus registration for Orion5x/Kirkwood


 arch/arm/plat-orion/common.c               |   54 ++++----
 arch/powerpc/platforms/chrp/pegasos_eth.c  |   20 +++
 arch/powerpc/sysdev/mv64x60_dev.c          |   16 ++-
 drivers/net/ethernet/marvell/Kconfig       |    5 +-
 drivers/net/ethernet/marvell/Makefile      |    2 +-
 drivers/net/ethernet/marvell/mv643xx_eth.c |  195 +++-------------------------
 include/linux/mv643xx_eth.h                |    1 -
 7 files changed, 84 insertions(+), 209 deletions(-)

diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index 2d4b641..251f827 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -238,6 +238,7 @@ static __init void ge_complete(
 	struct mv643xx_eth_shared_platform_data *orion_ge_shared_data,
 	struct resource *orion_ge_resource, unsigned long irq,
 	struct platform_device *orion_ge_shared,
+	struct platform_device *orion_ge_mvmdio,
 	struct mv643xx_eth_platform_data *eth_data,
 	struct platform_device *orion_ge)
 {
@@ -247,6 +248,8 @@ static __init void ge_complete(
 	orion_ge->dev.platform_data = eth_data;
 
 	platform_device_register(orion_ge_shared);
+	if (orion_ge_mvmdio)
+		platform_device_register(orion_ge_mvmdio);
 	platform_device_register(orion_ge);
 }
 
@@ -258,8 +261,6 @@ struct mv643xx_eth_shared_platform_data orion_ge00_shared_data;
 static struct resource orion_ge00_shared_resources[] = {
 	{
 		.name	= "ge00 base",
-	}, {
-		.name	= "ge00 err irq",
 	},
 };
 
@@ -271,6 +272,19 @@ static struct platform_device orion_ge00_shared = {
 	},
 };
 
+static struct resource orion_ge_mvmdio_resources[] = {
+	{
+		.name	= "ge00 mvmdio base",
+	}, {
+		.name	= "ge00 mvmdio err irq",
+	},
+};
+
+static struct platform_device orion_ge_mvmdio = {
+	.name		= "orion-mdio",
+	.id		= -1,
+};
+
 static struct resource orion_ge00_resources[] = {
 	{
 		.name	= "ge00 irq",
@@ -295,26 +309,25 @@ void __init orion_ge00_init(struct mv643xx_eth_platform_data *eth_data,
 			    unsigned int tx_csum_limit)
 {
 	fill_resources(&orion_ge00_shared, orion_ge00_shared_resources,
-		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+	fill_resources(&orion_ge_mvmdio, orion_ge_mvmdio_resources,
+			mapbase + 0x2004, 0x84 - 1, irq_err);
 	orion_ge00_shared_data.tx_csum_limit = tx_csum_limit;
 	ge_complete(&orion_ge00_shared_data,
 		    orion_ge00_resources, irq, &orion_ge00_shared,
+		    &orion_ge_mvmdio,
 		    eth_data, &orion_ge00);
 }
 
 /*****************************************************************************
  * GE01
  ****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge01_shared_data = {
-	.shared_smi	= &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge01_shared_data;
 
 static struct resource orion_ge01_shared_resources[] = {
 	{
 		.name	= "ge01 base",
-	}, {
-		.name	= "ge01 err irq",
-	},
+	}
 };
 
 static struct platform_device orion_ge01_shared = {
@@ -349,26 +362,23 @@ void __init orion_ge01_init(struct mv643xx_eth_platform_data *eth_data,
 			    unsigned int tx_csum_limit)
 {
 	fill_resources(&orion_ge01_shared, orion_ge01_shared_resources,
-		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
 	orion_ge01_shared_data.tx_csum_limit = tx_csum_limit;
 	ge_complete(&orion_ge01_shared_data,
 		    orion_ge01_resources, irq, &orion_ge01_shared,
+		    NULL,
 		    eth_data, &orion_ge01);
 }
 
 /*****************************************************************************
  * GE10
  ****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge10_shared_data = {
-	.shared_smi	= &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge10_shared_data;
 
 static struct resource orion_ge10_shared_resources[] = {
 	{
 		.name	= "ge10 base",
-	}, {
-		.name	= "ge10 err irq",
-	},
+	}
 };
 
 static struct platform_device orion_ge10_shared = {
@@ -402,24 +412,21 @@ void __init orion_ge10_init(struct mv643xx_eth_platform_data *eth_data,
 			    unsigned long irq_err)
 {
 	fill_resources(&orion_ge10_shared, orion_ge10_shared_resources,
-		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
 	ge_complete(&orion_ge10_shared_data,
 		    orion_ge10_resources, irq, &orion_ge10_shared,
+		    NULL,
 		    eth_data, &orion_ge10);
 }
 
 /*****************************************************************************
  * GE11
  ****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge11_shared_data = {
-	.shared_smi	= &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge11_shared_data;
 
 static struct resource orion_ge11_shared_resources[] = {
 	{
 		.name	= "ge11 base",
-	}, {
-		.name	= "ge11 err irq",
 	},
 };
 
@@ -454,9 +461,10 @@ void __init orion_ge11_init(struct mv643xx_eth_platform_data *eth_data,
 			    unsigned long irq_err)
 {
 	fill_resources(&orion_ge11_shared, orion_ge11_shared_resources,
-		       mapbase + 0x2000, SZ_16K - 1, irq_err);
+		       mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
 	ge_complete(&orion_ge11_shared_data,
 		    orion_ge11_resources, irq, &orion_ge11_shared,
+		    NULL,
 		    eth_data, &orion_ge11);
 }
 
diff --git a/arch/powerpc/platforms/chrp/pegasos_eth.c b/arch/powerpc/platforms/chrp/pegasos_eth.c
index 039fc8e..2b4dc6a 100644
--- a/arch/powerpc/platforms/chrp/pegasos_eth.c
+++ b/arch/powerpc/platforms/chrp/pegasos_eth.c
@@ -47,6 +47,25 @@ static struct platform_device mv643xx_eth_shared_device = {
 	.resource	= mv643xx_eth_shared_resources,
 };
 
+/*
+ * The orion mdio driver only covers shared + 0x4 up to shared + 0x84 - 1
+ */
+static struct resource mv643xx_eth_mvmdio_resources[] = {
+	[0] = {
+		.name	= "ethernet mdio base",
+		.start	= 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x4,
+		.end	= 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x83,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct platform_device mv643xx_eth_mvmdio_device = {
+	.name		= "orion-mdio",
+	.id		= -1,
+	.num_resources	= ARRAY_SIZE(mv643xx_eth_mvmdio_resources),
+	.resource	= mv643xx_eth_shared_resources,
+};
+
 static struct resource mv643xx_eth_port1_resources[] = {
 	[0] = {
 		.name	= "eth port1 irq",
@@ -82,6 +101,7 @@ static struct platform_device eth_port1_device = {
 
 static struct platform_device *mv643xx_eth_pd_devs[] __initdata = {
 	&mv643xx_eth_shared_device,
+	&mv643xx_eth_mvmdio_device,
 	&eth_port1_device,
 };
 
diff --git a/arch/powerpc/sysdev/mv64x60_dev.c b/arch/powerpc/sysdev/mv64x60_dev.c
index 0f6af41..4a25c26 100644
--- a/arch/powerpc/sysdev/mv64x60_dev.c
+++ b/arch/powerpc/sysdev/mv64x60_dev.c
@@ -214,15 +214,27 @@ static struct platform_device * __init mv64x60_eth_register_shared_pdev(
 						struct device_node *np, int id)
 {
 	struct platform_device *pdev;
-	struct resource r[1];
+	struct resource r[2];
 	int err;
 
 	err = of_address_to_resource(np, 0, &r[0]);
 	if (err)
 		return ERR_PTR(err);
 
+	/* register an orion mdio bus driver */
+	r[1].start = r[0].start + 0x4;
+	r[1].end = r[0].start + 0x84 - 1;
+	r[1].flags = IORESOURCE_MEM;
+
+	if (id == 0) {
+		pdev = platform_device_register_simple("orion-mdio", -1, &r[1], 1);
+		if (!pdev)
+			return pdev;
+	}
+
 	pdev = platform_device_register_simple(MV643XX_ETH_SHARED_NAME, id,
-					       r, 1);
+					       &r[0], 1);
+
 	return pdev;
 }
 
diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index edfba93..5170ecb 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -23,6 +23,7 @@ config MV643XX_ETH
 	depends on (MV64X60 || PPC32 || PLAT_ORION) && INET
 	select INET_LRO
 	select PHYLIB
+	select MVMDIO
 	---help---
 	  This driver supports the gigabit ethernet MACs in the
 	  Marvell Discovery PPC/MIPS chipset family (MV643XX) and
@@ -38,9 +39,7 @@ config MVMDIO
 	  interface units of the Marvell EBU SoCs (Kirkwood, Orion5x,
 	  Dove, Armada 370 and Armada XP).
 
-	  For now, this driver is only needed for the MVNETA driver
-	  (used on Armada 370 and XP), but it could be used in the
-	  future by the MV643XX_ETH driver.
+	  This driver is used by the MV643XX_ETH and MVNETA drivers.
 
 config MVNETA
 	tristate "Marvell Armada 370/XP network interface support"
diff --git a/drivers/net/ethernet/marvell/Makefile b/drivers/net/ethernet/marvell/Makefile
index 7f63b4a..5c4a776 100644
--- a/drivers/net/ethernet/marvell/Makefile
+++ b/drivers/net/ethernet/marvell/Makefile
@@ -2,8 +2,8 @@
 # Makefile for the Marvell device drivers.
 #
 
-obj-$(CONFIG_MV643XX_ETH) += mv643xx_eth.o
 obj-$(CONFIG_MVMDIO) += mvmdio.o
+obj-$(CONFIG_MV643XX_ETH) += mv643xx_eth.o
 obj-$(CONFIG_MVNETA) += mvneta.o
 obj-$(CONFIG_PXA168_ETH) += pxa168_eth.o
 obj-$(CONFIG_SKGE) += skge.o
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index d1ecf4b..a65a92e 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -69,14 +69,6 @@ static char mv643xx_eth_driver_version[] = "1.4";
  * Registers shared between all ports.
  */
 #define PHY_ADDR			0x0000
-#define SMI_REG				0x0004
-#define  SMI_BUSY			0x10000000
-#define  SMI_READ_VALID			0x08000000
-#define  SMI_OPCODE_READ		0x04000000
-#define  SMI_OPCODE_WRITE		0x00000000
-#define ERR_INT_CAUSE			0x0080
-#define  ERR_INT_SMI_DONE		0x00000010
-#define ERR_INT_MASK			0x0084
 #define WINDOW_BASE(w)			(0x0200 + ((w) << 3))
 #define WINDOW_SIZE(w)			(0x0204 + ((w) << 3))
 #define WINDOW_REMAP_HIGH(w)		(0x0280 + ((w) << 2))
@@ -266,25 +258,6 @@ struct mv643xx_eth_shared_private {
 	void __iomem *base;
 
 	/*
-	 * Points at the right SMI instance to use.
-	 */
-	struct mv643xx_eth_shared_private *smi;
-
-	/*
-	 * Provides access to local SMI interface.
-	 */
-	struct mii_bus *smi_bus;
-
-	/*
-	 * If we have access to the error interrupt pin (which is
-	 * somewhat misnamed as it not only reflects internal errors
-	 * but also reflects SMI completion), use that to wait for
-	 * SMI access completion instead of polling the SMI busy bit.
-	 */
-	int err_interrupt;
-	wait_queue_head_t smi_busy_wait;
-
-	/*
 	 * Per-port MBUS window access register value.
 	 */
 	u32 win_protect;
@@ -1122,97 +1095,6 @@ out_write:
 	wrlp(mp, PORT_SERIAL_CONTROL, pscr);
 }
 
-static irqreturn_t mv643xx_eth_err_irq(int irq, void *dev_id)
-{
-	struct mv643xx_eth_shared_private *msp = dev_id;
-
-	if (readl(msp->base + ERR_INT_CAUSE) & ERR_INT_SMI_DONE) {
-		writel(~ERR_INT_SMI_DONE, msp->base + ERR_INT_CAUSE);
-		wake_up(&msp->smi_busy_wait);
-		return IRQ_HANDLED;
-	}
-
-	return IRQ_NONE;
-}
-
-static int smi_is_done(struct mv643xx_eth_shared_private *msp)
-{
-	return !(readl(msp->base + SMI_REG) & SMI_BUSY);
-}
-
-static int smi_wait_ready(struct mv643xx_eth_shared_private *msp)
-{
-	if (msp->err_interrupt == NO_IRQ) {
-		int i;
-
-		for (i = 0; !smi_is_done(msp); i++) {
-			if (i == 10)
-				return -ETIMEDOUT;
-			msleep(10);
-		}
-
-		return 0;
-	}
-
-	if (!smi_is_done(msp)) {
-		wait_event_timeout(msp->smi_busy_wait, smi_is_done(msp),
-				   msecs_to_jiffies(100));
-		if (!smi_is_done(msp))
-			return -ETIMEDOUT;
-	}
-
-	return 0;
-}
-
-static int smi_bus_read(struct mii_bus *bus, int addr, int reg)
-{
-	struct mv643xx_eth_shared_private *msp = bus->priv;
-	void __iomem *smi_reg = msp->base + SMI_REG;
-	int ret;
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	writel(SMI_OPCODE_READ | (reg << 21) | (addr << 16), smi_reg);
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	ret = readl(smi_reg);
-	if (!(ret & SMI_READ_VALID)) {
-		pr_warn("SMI bus read not valid\n");
-		return -ENODEV;
-	}
-
-	return ret & 0xffff;
-}
-
-static int smi_bus_write(struct mii_bus *bus, int addr, int reg, u16 val)
-{
-	struct mv643xx_eth_shared_private *msp = bus->priv;
-	void __iomem *smi_reg = msp->base + SMI_REG;
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	writel(SMI_OPCODE_WRITE | (reg << 21) |
-		(addr << 16) | (val & 0xffff), smi_reg);
-
-	if (smi_wait_ready(msp)) {
-		pr_warn("SMI bus busy timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	return 0;
-}
-
-
 /* statistics ***************************************************************/
 static struct net_device_stats *mv643xx_eth_get_stats(struct net_device *dev)
 {
@@ -2688,47 +2570,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 		goto out_free;
 
 	/*
-	 * Set up and register SMI bus.
-	 */
-	if (pd == NULL || pd->shared_smi == NULL) {
-		msp->smi_bus = mdiobus_alloc();
-		if (msp->smi_bus == NULL)
-			goto out_unmap;
-
-		msp->smi_bus->priv = msp;
-		msp->smi_bus->name = "mv643xx_eth smi";
-		msp->smi_bus->read = smi_bus_read;
-		msp->smi_bus->write = smi_bus_write,
-		snprintf(msp->smi_bus->id, MII_BUS_ID_SIZE, "%s-%d",
-			pdev->name, pdev->id);
-		msp->smi_bus->parent = &pdev->dev;
-		msp->smi_bus->phy_mask = 0xffffffff;
-		if (mdiobus_register(msp->smi_bus) < 0)
-			goto out_free_mii_bus;
-		msp->smi = msp;
-	} else {
-		msp->smi = platform_get_drvdata(pd->shared_smi);
-	}
-
-	msp->err_interrupt = NO_IRQ;
-	init_waitqueue_head(&msp->smi_busy_wait);
-
-	/*
-	 * Check whether the error interrupt is hooked up.
-	 */
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (res != NULL) {
-		int err;
-
-		err = request_irq(res->start, mv643xx_eth_err_irq,
-				  IRQF_SHARED, "mv643xx_eth", msp);
-		if (!err) {
-			writel(ERR_INT_SMI_DONE, msp->base + ERR_INT_MASK);
-			msp->err_interrupt = res->start;
-		}
-	}
-
-	/*
 	 * (Re-)program MBUS remapping windows if we are asked to.
 	 */
 	dram = mv_mbus_dram_info();
@@ -2743,10 +2584,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 
 	return 0;
 
-out_free_mii_bus:
-	mdiobus_free(msp->smi_bus);
-out_unmap:
-	iounmap(msp->base);
 out_free:
 	kfree(msp);
 out:
@@ -2756,14 +2593,7 @@ out:
 static int mv643xx_eth_shared_remove(struct platform_device *pdev)
 {
 	struct mv643xx_eth_shared_private *msp = platform_get_drvdata(pdev);
-	struct mv643xx_eth_shared_platform_data *pd = pdev->dev.platform_data;
 
-	if (pd == NULL || pd->shared_smi == NULL) {
-		mdiobus_unregister(msp->smi_bus);
-		mdiobus_free(msp->smi_bus);
-	}
-	if (msp->err_interrupt != NO_IRQ)
-		free_irq(msp->err_interrupt, msp);
 	iounmap(msp->base);
 	kfree(msp);
 
@@ -2826,14 +2656,21 @@ static void set_params(struct mv643xx_eth_private *mp,
 	mp->txq_count = pd->tx_queue_count ? : 1;
 }
 
+static void mv643xx_eth_adjust_link(struct net_device *dev)
+{
+	struct mv643xx_eth_private *mp = netdev_priv(dev);
+
+	mv643xx_adjust_pscr(mp);
+}
+
 static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
 				   int phy_addr)
 {
-	struct mii_bus *bus = mp->shared->smi->smi_bus;
 	struct phy_device *phydev;
 	int start;
 	int num;
 	int i;
+	char phy_id[MII_BUS_ID_SIZE + 3];
 
 	if (phy_addr == MV643XX_ETH_PHY_ADDR_DEFAULT) {
 		start = phy_addr_get(mp) & 0x1f;
@@ -2843,17 +2680,19 @@ static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
 		num = 1;
 	}
 
+	/* Attempt to connect to the PHY using orion-mdio */
 	phydev = NULL;
 	for (i = 0; i < num; i++) {
 		int addr = (start + i) & 0x1f;
 
-		if (bus->phy_map[addr] == NULL)
-			mdiobus_scan(bus, addr);
+		snprintf(phy_id, sizeof(phy_id), PHY_ID_FMT,
+				"orion-mdio-mii", addr);
 
-		if (phydev == NULL) {
-			phydev = bus->phy_map[addr];
-			if (phydev != NULL)
-				phy_addr_set(mp, addr);
+		phydev = phy_connect(mp->dev, phy_id, mv643xx_eth_adjust_link,
+				PHY_INTERFACE_MODE_GMII);
+		if (!IS_ERR(phydev)) {
+			phy_addr_set(mp, addr);
+			break;
 		}
 	}
 
@@ -2866,8 +2705,6 @@ static void phy_init(struct mv643xx_eth_private *mp, int speed, int duplex)
 
 	phy_reset(mp);
 
-	phy_attach(mp->dev, dev_name(&phy->dev), PHY_INTERFACE_MODE_GMII);
-
 	if (speed == 0) {
 		phy->autoneg = AUTONEG_ENABLE;
 		phy->speed = 0;
diff --git a/include/linux/mv643xx_eth.h b/include/linux/mv643xx_eth.h
index 49258e0..141d395 100644
--- a/include/linux/mv643xx_eth.h
+++ b/include/linux/mv643xx_eth.h
@@ -19,7 +19,6 @@
 
 struct mv643xx_eth_shared_platform_data {
 	struct mbus_dram_target_info	*dram;
-	struct platform_device	*shared_smi;
 	/*
 	 * Max packet size for Tx IP/Layer 4 checksum, when set to 0, default
 	 * limit of 9KiB will be used.
-- 
1.7.10.4

Re: [PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver

[Thomas Petazzoni]

Hello,

On Fri, 22 Mar 2013 14:39:24 +0100, Florian Fainelli wrote:
> Hi all,
> 
> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
> instead of rolling its own implementation. As a result, all users of this
> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
> The mvmdio driver is also updated to support an interrupt line which reports
> SMI error/completion, and to allow traditionnal platform device registration
> instead of just device tree.
> 
> David, I think it makes sense for you to merge all of this, since we do
> not want the architecture files to be desynchronized from the mv643xx_eth to
> avoid runtime breakage. The potential for merge conflicts should be very small.
> 
> Florian Fainelli (4):
>   net: mvmdio: allow platform device style registration
>   net: mvmdio: rename base register cookie from smireg to regs
>   net: mvmdio: enhance driver to support SMI error/done interrupts
>   mv643xx_eth: convert to use the Marvell Orion MDIO driver

For the entire series:

Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

tested on:

 * Armada XP DB, DT-based, which uses the mvneta driver. It is affected
   by the 3 first commits since Armada XP also uses the mvmdio driver.

 * Kirkwood development board, non-DT, which uses the mv643xx_eth
   driver.

Thanks Florian for this work,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver

[Jason Cooper]

On Fri, Mar 22, 2013 at 02:39:24PM +0100, Florian Fainelli wrote:
> Hi all,
> 
> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
> instead of rolling its own implementation. As a result, all users of this
> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
> The mvmdio driver is also updated to support an interrupt line which reports
> SMI error/completion, and to allow traditionnal platform device registration
> instead of just device tree.
> 
> David, I think it makes sense for you to merge all of this, since we do
> not want the architecture files to be desynchronized from the mv643xx_eth to
> avoid runtime breakage. The potential for merge conflicts should be very small.
> 
> Florian Fainelli (4):
>   net: mvmdio: allow platform device style registration
>   net: mvmdio: rename base register cookie from smireg to regs
>   net: mvmdio: enhance driver to support SMI error/done interrupts
>   mv643xx_eth: convert to use the Marvell Orion MDIO driver
> 
>  .../devicetree/bindings/net/marvell-orion-mdio.txt |    3 +
>  arch/arm/plat-orion/common.c                       |   54 +++---
>  arch/powerpc/platforms/chrp/pegasos_eth.c          |   20 ++
>  arch/powerpc/sysdev/mv64x60_dev.c                  |   16 +-
>  drivers/net/ethernet/marvell/Kconfig               |    5 +-
>  drivers/net/ethernet/marvell/Makefile              |    2 +-
>  drivers/net/ethernet/marvell/mv643xx_eth.c         |  195 ++------------------
>  drivers/net/ethernet/marvell/mvmdio.c              |  130 ++++++++++---
>  include/linux/mv643xx_eth.h                        |    1 -
>  9 files changed, 187 insertions(+), 239 deletions(-)

Whole series applied on top of v3.9-rc3 and tested on dreamplug
(kirkwood DT boot with legacy mv643xx_eth init)

Tested-by: Jason Cooper <jason@lakedaemon.net>

also, for the bits changing plat-orion:

Acked-by: Jason Cooper <jason@lakedaemon.net>

thx,

Jason.

Re: [PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver

[Florian Fainelli]

Le 03/22/13 15:14, Jason Cooper a écrit :
> On Fri, Mar 22, 2013 at 02:39:24PM +0100, Florian Fainelli wrote:
>> Hi all,
>>
>> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
>> instead of rolling its own implementation. As a result, all users of this
>> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
>> The mvmdio driver is also updated to support an interrupt line which reports
>> SMI error/completion, and to allow traditionnal platform device registration
>> instead of just device tree.
>>
>> David, I think it makes sense for you to merge all of this, since we do
>> not want the architecture files to be desynchronized from the mv643xx_eth to
>> avoid runtime breakage. The potential for merge conflicts should be very small.
>>
>> Florian Fainelli (4):
>>    net: mvmdio: allow platform device style registration
>>    net: mvmdio: rename base register cookie from smireg to regs
>>    net: mvmdio: enhance driver to support SMI error/done interrupts
>>    mv643xx_eth: convert to use the Marvell Orion MDIO driver
>>
>>   .../devicetree/bindings/net/marvell-orion-mdio.txt |    3 +
>>   arch/arm/plat-orion/common.c                       |   54 +++---
>>   arch/powerpc/platforms/chrp/pegasos_eth.c          |   20 ++
>>   arch/powerpc/sysdev/mv64x60_dev.c                  |   16 +-
>>   drivers/net/ethernet/marvell/Kconfig               |    5 +-
>>   drivers/net/ethernet/marvell/Makefile              |    2 +-
>>   drivers/net/ethernet/marvell/mv643xx_eth.c         |  195 ++------------------
>>   drivers/net/ethernet/marvell/mvmdio.c              |  130 ++++++++++---
>>   include/linux/mv643xx_eth.h                        |    1 -
>>   9 files changed, 187 insertions(+), 239 deletions(-)
>
> Whole series applied on top of v3.9-rc3 and tested on dreamplug
> (kirkwood DT boot with legacy mv643xx_eth init)

Ok, thanks! Does that mean that you want these changes to go via your 
tree? David initially applied my v2 of this patchset, and since it 
thouches mostly ethernet driver stuff, I would rather make it go via his 
tree if both of you agree.
--
Florian

Re: [PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver

[Jason Cooper]

On Fri, Mar 22, 2013 at 03:24:55PM +0100, Florian Fainelli wrote:
> Le 03/22/13 15:14, Jason Cooper a écrit :
> >On Fri, Mar 22, 2013 at 02:39:24PM +0100, Florian Fainelli wrote:
> >>Hi all,
> >>
> >>This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
> >>instead of rolling its own implementation. As a result, all users of this
> >>mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
> >>The mvmdio driver is also updated to support an interrupt line which reports
> >>SMI error/completion, and to allow traditionnal platform device registration
> >>instead of just device tree.
> >>
> >>David, I think it makes sense for you to merge all of this, since we do
> >>not want the architecture files to be desynchronized from the mv643xx_eth to
> >>avoid runtime breakage. The potential for merge conflicts should be very small.
> >>
> >>Florian Fainelli (4):
> >>   net: mvmdio: allow platform device style registration
> >>   net: mvmdio: rename base register cookie from smireg to regs
> >>   net: mvmdio: enhance driver to support SMI error/done interrupts
> >>   mv643xx_eth: convert to use the Marvell Orion MDIO driver
> >>
> >>  .../devicetree/bindings/net/marvell-orion-mdio.txt |    3 +
> >>  arch/arm/plat-orion/common.c                       |   54 +++---
> >>  arch/powerpc/platforms/chrp/pegasos_eth.c          |   20 ++
> >>  arch/powerpc/sysdev/mv64x60_dev.c                  |   16 +-
> >>  drivers/net/ethernet/marvell/Kconfig               |    5 +-
> >>  drivers/net/ethernet/marvell/Makefile              |    2 +-
> >>  drivers/net/ethernet/marvell/mv643xx_eth.c         |  195 ++------------------
> >>  drivers/net/ethernet/marvell/mvmdio.c              |  130 ++++++++++---
> >>  include/linux/mv643xx_eth.h                        |    1 -
> >>  9 files changed, 187 insertions(+), 239 deletions(-)
> >
> >Whole series applied on top of v3.9-rc3 and tested on dreamplug
> >(kirkwood DT boot with legacy mv643xx_eth init)
> 
> Ok, thanks! Does that mean that you want these changes to go via
> your tree? David initially applied my v2 of this patchset, and since
> it thouches mostly ethernet driver stuff, I would rather make it go
> via his tree if both of you agree.

Yeah, I thought I should have reworded that after I hit send :)  I
simply meant it applied cleanly against v3.9-rc3, booted, and worked.  I
Acked it so David could take the whole series through his tree.  Sorry
for the confusion.

Now that I can build mv643xx_eth DT on top of this, I'll structure it so
those changes go on top of yours (in David's tree) and try to avoid the
external dependency for the DT bits going though arm-soc.

thx,

Jason.

Re: [PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver

[Florian Fainelli]

Le 03/22/13 15:29, Jason Cooper a écrit :
>> Ok, thanks! Does that mean that you want these changes to go via
>> your tree? David initially applied my v2 of this patchset, and since
>> it thouches mostly ethernet driver stuff, I would rather make it go
>> via his tree if both of you agree.
>
> Yeah, I thought I should have reworded that after I hit send :)  I
> simply meant it applied cleanly against v3.9-rc3, booted, and worked.  I
> Acked it so David could take the whole series through his tree.  Sorry
> for the confusion.
>
> Now that I can build mv643xx_eth DT on top of this, I'll structure it so
> those changes go on top of yours (in David's tree) and try to avoid the
> external dependency for the DT bits going though arm-soc.

Sounds good! FYI, I am finishing up DSA Device Tree bindings so that we 
can finally get rid of board-specific Kirkwood files.
--
Florian

Re: [PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver

[David Miller]

From: Florian Fainelli <florian@openwrt.org>
Date: Fri, 22 Mar 2013 14:39:24 +0100

> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
> instead of rolling its own implementation. As a result, all users of this
> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
> The mvmdio driver is also updated to support an interrupt line which reports
> SMI error/completion, and to allow traditionnal platform device registration
> instead of just device tree.
> 
> David, I think it makes sense for you to merge all of this, since we do
> not want the architecture files to be desynchronized from the mv643xx_eth to
> avoid runtime breakage. The potential for merge conflicts should be very small.

Series applied to net-next, thanks Florian.