[PATCH 0/6] mvmdio updates

[PATCH 0/6] mvmdio updates

[Russell King - ARM Linux]

This series of patches update mvmdio for Armada 8k CP110.  A number of
issues were found:

1. The driver fails to disable an interrupt when something goes wrong
   in the probe function.

2. The interrupt is specified in DT to be optional, but the driver
   unconditionally writes to the interrupt mask register, which may
   not exist.

3. The DT binding specifies
    "reg: address and length of the SMI register"
   however, when supporting the interrupt, the size must cover the
   interrupt register as well.  Update the binding documentation
   with this information that was previously omitted.

4. If the register size is too small, have the driver print an error
   and disable use of the interrupt.

5. Armada 8k needs three clocks for the MDIO interface, otherwise the
   SoC hangs (since it is part of one of the ethernet interfaces.)
   GOP clock, MG core clock and MG clock are needed on 8k. Augment the
   binding and driver to allow three clocks to be specified.

 .../devicetree/bindings/net/marvell-orion-mdio.txt | 19 ++++++++--
 drivers/net/ethernet/marvell/mvmdio.c              | 44 +++++++++++++++++-----
 2 files changed, 50 insertions(+), 13 deletions(-)

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

[PATCH 1/6] net: mvmdio: disable interrupts in driver failure path

[Russell King]

When the mvmdio driver has an interrupt, it enables the "done" interrupt
after requesting its interrupt handler.  However, probe failure results
in the interrupt being left enabled.  Disable it on the failure path.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvmdio.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index a0d1b084ecec..7aea0beca56e 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -251,6 +251,8 @@ static int orion_mdio_probe(struct platform_device *pdev)
 	return 0;
 
 out_mdio:
+	if (dev->err_interrupt > 0)
+		writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
 	if (!IS_ERR(dev->clk))
 		clk_disable_unprepare(dev->clk);
 	return ret;
-- 
2.7.4

[PATCH 2/6] net: mvmdio: fix interrupt disable in remove path

[Russell King]

The pre-existing write to disable interrupts on the remove path happens
whether we have an interrupt or not.  While this may seem to be a good
idea, this driver is re-used in many different implementations, some
where the binding only specifies four bytes of register space.  This
access causes us to access registers outside of the binding.

Make it conditional on the interrupt being present, which is the same
condition used when enabling the interrupt in the first place.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvmdio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 7aea0beca56e..6ea5caddca62 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -263,7 +263,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);
+	if (dev->err_interrupt > 0)
+		writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
 	mdiobus_unregister(bus);
 	if (!IS_ERR(dev->clk))
 		clk_disable_unprepare(dev->clk);
-- 
2.7.4

[PATCH 3/6] dt-bindings: correct marvell orion MDIO binding document

[Russell King]

Correct the Marvell Orion MDIO binding document to properly reflect the
cases where an interrupt is present.  Augment the examples to show this.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 .../devicetree/bindings/net/marvell-orion-mdio.txt      | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
index 9417e54c26c0..ca733ff68ab9 100644
--- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
+++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
@@ -7,7 +7,10 @@ interface.
 
 Required properties:
 - compatible: "marvell,orion-mdio"
-- reg: address and length of the SMI register
+- reg: address and length of the MDIO registers.  When an interrupt is
+  not present, the length is the size of the SMI register (4 bytes)
+  otherwise it must be 0x84 bytes to cover the interrupt control
+  registers.
 
 Optional properties:
 - interrupts: interrupt line number for the SMI error/done interrupt
@@ -17,7 +20,7 @@ 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.
 
-Example at the SoC level:
+Example at the SoC level without an interrupt property:
 
 mdio {
 	#address-cells = <1>;
@@ -26,6 +29,16 @@ mdio {
 	reg = <0xd0072004 0x4>;
 };
 
+Example with an interrupt property:
+
+mdio {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	compatible = "marvell,orion-mdio";
+	reg = <0xd0072004 0x84>;
+	interrupts = <30>;
+};
+
 And at the board level:
 
 mdio {
-- 
2.7.4

[PATCH 4/6] net: mvmdio: disable interrupt if resource size is too small

[Russell King]

Disable the MDIO interrupt, falling back to polled mode, if the resource
size does not allow us to access the interrupt registers.  All current
DT bindings use a size of 0x84, which allows access, but verifying it is
good practice.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvmdio.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 6ea5caddca62..614dfde657fe 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -221,6 +221,12 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		clk_prepare_enable(dev->clk);
 
 	dev->err_interrupt = platform_get_irq(pdev, 0);
+	if (dev->err_interrupt > 0 &&
+	    resource_size(r) < MVMDIO_ERR_INT_MASK + 4) {
+		dev_err(&pdev->dev,
+			"disabling interrupt, resource size is too small\n");
+		dev->err_interrupt = 0;
+	}
 	if (dev->err_interrupt > 0) {
 		ret = devm_request_irq(&pdev->dev, dev->err_interrupt,
 					orion_mdio_err_irq,
-- 
2.7.4

[PATCH 5/6] dt-bindings: allow up to three clocks for orion-mdio

[Russell King]

Armada 8040 needs three clocks to be enabled for MDIO accesses to work.
Update the binding to allow the extra clocks to be specified.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 Documentation/devicetree/bindings/net/marvell-orion-mdio.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
index ca733ff68ab9..ccdabdcc8618 100644
--- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
+++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
@@ -14,7 +14,7 @@ interface.
 
 Optional properties:
 - interrupts: interrupt line number for the SMI error/done interrupt
-- clocks: Phandle to the clock control device and gate bit
+- clocks: phandle for up to three required clocks for the MDIO instance
 
 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
-- 
2.7.4

[PATCH 6/6] net: mvmdio: allow up to three clocks to be specified for orion-mdio

[Russell King]

Allow up to three clocks to be specified and enabled for the orion-mdio
interface, which are required for this interface to be accessible on
Armada 8k platforms.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvmdio.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 614dfde657fe..90a60b98c28e 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -53,7 +53,7 @@
 struct orion_mdio_dev {
 	struct mutex lock;
 	void __iomem *regs;
-	struct clk *clk;
+	struct clk *clk[3];
 	/*
 	 * If we have access to the error interrupt pin (which is
 	 * somewhat misnamed as it not only reflects internal errors
@@ -187,7 +187,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
 	struct resource *r;
 	struct mii_bus *bus;
 	struct orion_mdio_dev *dev;
-	int ret;
+	int i, ret;
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!r) {
@@ -216,9 +216,12 @@ static int orion_mdio_probe(struct platform_device *pdev)
 
 	init_waitqueue_head(&dev->smi_busy_wait);
 
-	dev->clk = devm_clk_get(&pdev->dev, NULL);
-	if (!IS_ERR(dev->clk))
-		clk_prepare_enable(dev->clk);
+	for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
+		dev->clk[i] = of_clk_get(pdev->dev.of_node, i);
+		if (IS_ERR(dev->clk[i]))
+			break;
+		clk_prepare_enable(dev->clk[i]);
+	}
 
 	dev->err_interrupt = platform_get_irq(pdev, 0);
 	if (dev->err_interrupt > 0 &&
@@ -259,8 +262,14 @@ static int orion_mdio_probe(struct platform_device *pdev)
 out_mdio:
 	if (dev->err_interrupt > 0)
 		writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
-	if (!IS_ERR(dev->clk))
-		clk_disable_unprepare(dev->clk);
+
+	for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
+		if (IS_ERR(dev->clk[i]))
+			break;
+		clk_disable_unprepare(dev->clk[i]);
+		clk_put(dev->clk[i]);
+	}
+
 	return ret;
 }
 
@@ -268,12 +277,18 @@ static int orion_mdio_remove(struct platform_device *pdev)
 {
 	struct mii_bus *bus = platform_get_drvdata(pdev);
 	struct orion_mdio_dev *dev = bus->priv;
+	int i;
 
 	if (dev->err_interrupt > 0)
 		writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
 	mdiobus_unregister(bus);
-	if (!IS_ERR(dev->clk))
-		clk_disable_unprepare(dev->clk);
+
+	for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
+		if (IS_ERR(dev->clk[i]))
+			break;
+		clk_disable_unprepare(dev->clk[i]);
+		clk_put(dev->clk[i]);
+	}
 
 	return 0;
 }
-- 
2.7.4

Re: [PATCH 1/6] net: mvmdio: disable interrupts in driver failure path

[Andrew Lunn]

On Mon, Apr 10, 2017 at 04:28:04PM +0100, Russell King wrote:
> When the mvmdio driver has an interrupt, it enables the "done" interrupt
> after requesting its interrupt handler.  However, probe failure results
> in the interrupt being left enabled.  Disable it on the failure path.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH 2/6] net: mvmdio: fix interrupt disable in remove path

[Andrew Lunn]

On Mon, Apr 10, 2017 at 04:28:09PM +0100, Russell King wrote:
> The pre-existing write to disable interrupts on the remove path happens
> whether we have an interrupt or not.  While this may seem to be a good
> idea, this driver is re-used in many different implementations, some
> where the binding only specifies four bytes of register space.  This
> access causes us to access registers outside of the binding.
> 
> Make it conditional on the interrupt being present, which is the same
> condition used when enabling the interrupt in the first place.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH 3/6] dt-bindings: correct marvell orion MDIO binding document

[Andrew Lunn]

On Mon, Apr 10, 2017 at 04:28:15PM +0100, Russell King wrote:
> Correct the Marvell Orion MDIO binding document to properly reflect the
> cases where an interrupt is present.  Augment the examples to show this.
> 
> Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>

Reviewed-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>

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

Re: [PATCH 4/6] net: mvmdio: disable interrupt if resource size is too small

[Andrew Lunn]

On Mon, Apr 10, 2017 at 04:28:20PM +0100, Russell King wrote:
> Disable the MDIO interrupt, falling back to polled mode, if the resource
> size does not allow us to access the interrupt registers.  All current
> DT bindings use a size of 0x84, which allows access, but verifying it is
> good practice.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH 6/6] net: mvmdio: allow up to three clocks to be specified for orion-mdio

[Andrew Lunn]

On Mon, Apr 10, 2017 at 04:28:31PM +0100, Russell King wrote:
> Allow up to three clocks to be specified and enabled for the orion-mdio
> interface, which are required for this interface to be accessible on
> Armada 8k platforms.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH 0/6] mvmdio updates

[Marcin Wojtas]

Hi Russel,

2017-04-10 17:27 GMT+02:00 Russell King - ARM Linux <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>:
> This series of patches update mvmdio for Armada 8k CP110.  A number of
> issues were found:
>
> 1. The driver fails to disable an interrupt when something goes wrong
>    in the probe function.
>
> 2. The interrupt is specified in DT to be optional, but the driver
>    unconditionally writes to the interrupt mask register, which may
>    not exist.
>
> 3. The DT binding specifies
>     "reg: address and length of the SMI register"
>    however, when supporting the interrupt, the size must cover the
>    interrupt register as well.  Update the binding documentation
>    with this information that was previously omitted.
>
> 4. If the register size is too small, have the driver print an error
>    and disable use of the interrupt.
>
> 5. Armada 8k needs three clocks for the MDIO interface, otherwise the
>    SoC hangs (since it is part of one of the ethernet interfaces.)
>    GOP clock, MG core clock and MG clock are needed on 8k. Augment the
>    binding and driver to allow three clocks to be specified.
>

Actually most of the interfaces on a7k/a8k require multiple clocks to
be enabled, however all those twisted dependencies are handled within:
drivers/clk/mvebu/cp110-system-controller.c
With the latest patch of Thomas Petazzoni, MG clock is already
specified as a child of MG_CORE, so I believe a just minor change will
resolve remaining GOP clock dependency. This way we will leave
orion-mdio driver untouched around clocks.

Thomas, what is your opinion?

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

Re: [PATCH 5/6] dt-bindings: allow up to three clocks for orion-mdio

[Andrew Lunn]

On Mon, Apr 10, 2017 at 04:28:25PM +0100, Russell King wrote:
> Armada 8040 needs three clocks to be enabled for MDIO accesses to work.
> Update the binding to allow the extra clocks to be specified.
> 
> Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>

Reviewed-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>

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

Re: [PATCH 0/6] mvmdio updates

[David Miller]

From: Russell King - ARM Linux <linux@armlinux.org.uk>
Date: Mon, 10 Apr 2017 16:27:28 +0100

> This series of patches update mvmdio for Armada 8k CP110.  A number of
> issues were found:
 ...

Series applied to net-next, thanks.