mv643xx Ethernet DT support and CSB1724 board support.

Hi Ian

Review

Quoted 6 lines

(quoted diff) +0 -0

b/arch/arm/configs/csb1724_defconfig new file mode 100644 index 0000000..927b269 --- /dev/null +++ b/arch/arm/configs/csb1724_defconfig @@ -0,0 +1,47 @@

(quoted diff) +2 -0

+CONFIG_ARCH_KIRKWOOD=y +CONFIG_ARCH_KIRKWOOD_DT=y
+CONFIG_MACH_CSB1724_DT=y

Do you really need this? Its the only kirkwood board with its on _defconfig file. What makes this board special?

Quoted 10 lines · Andrew Lunn

Hi Ian

Review

Quoted 6 lines

(quoted diff) +0 -0

b/arch/arm/configs/csb1724_defconfig new file mode 100644 index 0000000..927b269 --- /dev/null +++ b/arch/arm/configs/csb1724_defconfig @@ -0,0 +1,47 @@

(quoted diff) +2 -0

+CONFIG_ARCH_KIRKWOOD=y +CONFIG_ARCH_KIRKWOOD_DT=y
+CONFIG_MACH_CSB1724_DT=y

Do you really need this? Its the only kirkwood board with its on _defconfig file. What makes this board special?

Nothing. defconfigs are just there to make configuring your kernel easy.

The kirkwood defconfig doesnt fit the currently supported hardware set on the csb1724 very well. Why build stuff you dont need?

This patch adds basic device tree support to the mv643xx ethernet driver.

It should be enough for most current users of the device, and should allow a fairly painless migration once proper support for clk devices is available to those platforms.

Signed-off-by Ian Molton <[email protected]>

---
 drivers/net/ethernet/marvell/mv643xx_eth.c |  111 ++++++++++++++++++++++++----
 1 file changed, 97 insertions(+), 14 deletions(-)

Quoted 11 lines · Ian Molton

This patch adds basic device tree support to the mv643xx ethernet driver.

It should be enough for most current users of the device, and should allow a fairly painless migration once proper support for clk devices is available to those platforms.

Signed-off-by Ian Molton <[email protected]> ---

drivers/net/ethernet/marvell/mv643xx_eth.c | 111 ++++++++++++++++++++++++---- 1 file changed, 97 insertions(+), 14 deletions(-)

Hi Ian

Please document the DT binding in
Documentation/devicetree/binding/....

Also, this should also be CC: to the netdev mailing. I got into trouble for not doing this and breaking the PPC build :-(

This patch adds basic device tree support to the mv643xx ethernet driver.

It should be enough for most current users of the device, and should allow a fairly painless migration once proper support for clk devices is available
to those platforms.

Signed-off-by Ian Molton <[email protected]>

---
 drivers/net/ethernet/marvell/mv643xx_eth.c |  111
++++++++++++++++++++++++----
 1 file changed, 97 insertions(+), 14 deletions(-)

drivers/net/ethernet/marvell/mv643xx_eth.c +5 -2

index 92497eb..579692f 100644
@@ -48,6 +48,9 @@
 #include <linux/ethtool.h>
 #include <linux/platform_device.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/of_irq.h>
 #include <linux/kernel.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>

@@ -2601,11 +2604,11 @@ static void infer_hw_params(struct
 mv643xx_eth_shared_private *msp)
 static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 {
        static int mv643xx_eth_version_printed;
-       struct mv643xx_eth_shared_platform_data *pd =
 pdev->dev.platform_data;
+       struct mv643xx_eth_shared_platform_data *pd;
        struct mv643xx_eth_shared_private *msp;
        const struct mbus_dram_target_info *dram;
        struct resource *res;
-       int ret;
+       int ret, irq = -1;
 

        if (!mv643xx_eth_version_printed++)
                pr_notice("MV-643xx 10/100/1000 ethernet driver version

%s\n",

drivers/net/ethernet/marvell/mv643xx_eth.c +4 -0

@@ -2625,6 +2628,23 @@ static int mv643xx_eth_shared_probe(struct
 platform_device *pdev)
        if (msp->base == NULL)
                goto out_free;
 
+       if (pdev->dev.of_node) {
+               struct device_node *np = NULL;
+
+               /* when all users of this driver use FDT, we can remove

this */

drivers/net/ethernet/marvell/mv643xx_eth.c +3 -0

+               pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
+               if (!pd) {
+                       dev_dbg(&pdev->dev, "Could not allocate platform

data\n");

drivers/net/ethernet/marvell/mv643xx_eth.c +25 -4

+                       goto out_free;
+               }
+
+               np = of_parse_phandle(pdev->dev.of_node, "shared_smi", 0);
+               if (np)
+                       pd->shared_smi = of_find_device_by_node(np);
+
+       } else {
+               pd = pdev->dev.platform_data;
+       }
        /*
         * Set up and register SMI bus.
         */

@@ -2654,15 +2674,22 @@ static int mv643xx_eth_shared_probe(struct
 platform_device *pdev)
        /*
         * Check whether the error interrupt is hooked up.
         */
-       res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-       if (res != NULL) {
+       if (pdev->dev.of_node) {
+               irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+       } else {
+               res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+               if (res)
+                       irq = res->start;
+       }
+
+       if (irq != -1) {
                int err;
 
-               err = request_irq(res->start, mv643xx_eth_err_irq,
+               err = request_irq(irq, 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;
+                       msp->err_interrupt = irq;
                }
        }

@@ -2675,6 +2702,10 @@ static int mv643xx_eth_shared_probe(struct
 platform_device *pdev)
 
        msp->tx_csum_limit = (pd != NULL && pd->tx_csum_limit) ?
                                        pd->tx_csum_limit : 9 * 1024;
+
+       if (pdev->dev.of_node)
+               kfree(pd);  /* If we created a fake pd, free it now */
+
        infer_hw_params(msp);
 

        platform_set_drvdata(pdev, msp);

drivers/net/ethernet/marvell/mv643xx_eth.c +13 -1

@@ -2708,12 +2739,21 @@ static int mv643xx_eth_shared_remove(struct
 platform_device *pdev)
        return 0;
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id mv_mdio_dt_ids[] __devinitdata = {
+       { .compatible = "marvell,mdio-mv643xx", },
+       {},
+};
+MODULE_DEVICE_TABLE(of, mv_mdio_dt_ids);
+#endif
+
 static struct platform_driver mv643xx_eth_shared_driver = {
        .probe          = mv643xx_eth_shared_probe,
        .remove         = mv643xx_eth_shared_remove,
        .driver = {
                .name   = MV643XX_ETH_SHARED_NAME,
                .owner  = THIS_MODULE,
+               .of_match_table = of_match_ptr(mv_mdio_dt_ids),
        },
 };

@@ -2873,7 +2913,31 @@ static int mv643xx_eth_probe(struct platform_device
 *pdev)
        struct resource *res;
        int err;
 
-       pd = pdev->dev.platform_data;
+       if (pdev->dev.of_node) {
+               struct device_node *np = NULL;
+
+               /* when all users of this driver use FDT, we can remove

this */

drivers/net/ethernet/marvell/mv643xx_eth.c +3 -0

+               pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
+               if (!pd) {
+                       dev_dbg(&pdev->dev, "Could not allocate platform

data\n");

drivers/net/ethernet/marvell/mv643xx_eth.c +11 -0

+                       return -ENOMEM;
+               }
+
+               of_property_read_u32(pdev->dev.of_node,
+                       "port_number", &pd->port_number);
+               of_property_read_u32(pdev->dev.of_node,
+                       "phy_addr", &pd->phy_addr);
+               np = of_parse_phandle(pdev->dev.of_node, "mdio", 0);
+               if (np) {
+                       pd->shared = of_find_device_by_node(np);
+               } else {

drivers/net/ethernet/marvell/mv643xx_eth.c +6 -3

@@ -2881,12 +2945,15 @@ static int mv643xx_eth_probe(struct
 platform_device *pdev)
 
        if (pd->shared == NULL) {
                dev_err(&pdev->dev, "no
 mv643xx_eth_platform_data->shared\n");
-               return -ENODEV;
+               err = -ENODEV;
+               goto out_free_pd;
        }
 
        dev = alloc_etherdev_mq(sizeof(struct mv643xx_eth_private), 8);
-       if (!dev)
-               return -ENOMEM;
+       if (!dev) {
+               err = -ENOMEM;
+               goto out_free_pd;
+       }
 

        mp = netdev_priv(dev);
        platform_set_drvdata(pdev, mp);

drivers/net/ethernet/marvell/mv643xx_eth.c +19 -4

@@ -2923,6 +2990,8 @@ static int mv643xx_eth_probe(struct platform_device
 *pdev)
 
        init_pscr(mp, pd->speed, pd->duplex);
 
+       if (pdev->dev.of_node)
+               kfree(pd); /* If we created a fake pd, free it now */
 
        mib_counters_clear(mp);

@@ -2942,10 +3011,13 @@ static int mv643xx_eth_probe(struct
 platform_device *pdev)
        mp->rx_oom.data = (unsigned long)mp;
        mp->rx_oom.function = oom_timer_wrapper;
 
-
-       res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-       BUG_ON(!res);
-       dev->irq = res->start;
+       if (pdev->dev.of_node) {
+               dev->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+       } else {
+               res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+               BUG_ON(!res);
+               dev->irq = res->start;
+       }
 
        dev->netdev_ops = &mv643xx_eth_netdev_ops;

@@ -2991,6 +3063,8 @@ out:
        }
 #endif
        free_netdev(dev);
+out_free_pd:
+       kfree(pd);
 
        return err;
 }

@@ -3030,6 +3104,14 @@ static void mv643xx_eth_shutdown(struct
 platform_device *pdev)
                port_reset(mp);
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id mv_eth_dt_ids[] __devinitdata = {
+       { .compatible = "marvell,mv643xx", },
+       {},
+};
+MODULE_DEVICE_TABLE(of, mv_eth_dt_ids);
+#endif
+
 static struct platform_driver mv643xx_eth_driver = {
        .probe          = mv643xx_eth_probe,

.remove = mv643xx_eth_remove,

drivers/net/ethernet/marvell/mv643xx_eth.c +1 -1

@@ -3037,6 +3119,7 @@ static struct platform_driver mv643xx_eth_driver = {
        .driver = {
                .name   = MV643XX_ETH_NAME,
                .owner  = THIS_MODULE,
+               .of_match_table = of_match_ptr(mv_eth_dt_ids),
        },
 };
 
--

1.7.9.5

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Ian,

Quoted 27 lines · Ian Molton

drivers/net/ethernet/marvell/mv643xx_eth.c +4 -1

-       pd = pdev->dev.platform_data;
+       if (pdev->dev.of_node) {
+               struct device_node *np = NULL;
+
+               /* when all users of this driver use FDT, we can remove

this */

drivers/net/ethernet/marvell/mv643xx_eth.c +3 -0

+               pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
+               if (!pd) {
+                       dev_dbg(&pdev->dev, "Could not allocate platform

data\n");

drivers/net/ethernet/marvell/mv643xx_eth.c +17 -0

+                       return -ENOMEM;
+               }
+
+               of_property_read_u32(pdev->dev.of_node,
+                       "port_number", &pd->port_number);
+               of_property_read_u32(pdev->dev.of_node,
+                       "phy_addr", &pd->phy_addr);
+               np = of_parse_phandle(pdev->dev.of_node, "mdio", 0);
+               if (np) {
+                       pd->shared = of_find_device_by_node(np);
+               } else {
+                       kfree(pd);
+                       return -ENODEV;
+               }
+       } else {
+               pd = pdev->dev.platform_data;
+       }

Quoted 5 lines

+

        if (pd == NULL) {
                dev_err(&pdev->dev, "no mv643xx_eth_platform_data\n");
                return -ENODEV;

Can this check for pd be moved in the else part above as well, as for the pd allocation with kzalloc,
we have already made a check for memory allocation failure?

Quoted 3 lines · Amar Nath

Hi Ian,

On Mon, Jul 30, 2012 at 8:45 PM, Ian Molton

Quoted 8 lines

Quoted 8 lines

drivers/net/ethernet/marvell/mv643xx_eth.c +4 -1

-       pd = pdev->dev.platform_data;
+       if (pdev->dev.of_node) {
+               struct device_node *np = NULL;
+
+               /* when all users of this driver use FDT, we can remove

this */
+ pd = kzalloc(sizeof(*pd), GFP_ATOMIC);

Quoted 20 lines

Quoted 20 lines

(quoted diff) +2 -0

+               if (!pd) {
+                       dev_dbg(&pdev->dev, "Could not allocate platform

data\n");

drivers/net/ethernet/marvell/mv643xx_eth.c +17 -0

+                       return -ENOMEM;
+               }
+
+               of_property_read_u32(pdev->dev.of_node,
+                       "port_number", &pd->port_number);
+               of_property_read_u32(pdev->dev.of_node,
+                       "phy_addr", &pd->phy_addr);
+               np = of_parse_phandle(pdev->dev.of_node, "mdio", 0);
+               if (np) {
+                       pd->shared = of_find_device_by_node(np);
+               } else {
+                       kfree(pd);
+                       return -ENODEV;
+               }
+       } else {
+               pd = pdev->dev.platform_data;
+       }

Quoted 8 lines

Quoted 5 lines

+

        if (pd == NULL) {
                dev_err(&pdev->dev, "no mv643xx_eth_platform_data\n");
                return -ENODEV;

Can this check for pd be moved in the else part above as well, as for the pd allocation with kzalloc,
we have already made a check for memory allocation failure?

Yes, Folded in for v2.

-Ian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

drivers/net/ethernet/marvell/mv643xx_eth.c +4 -1

@@ -2873,7 +2913,31 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 	struct resource *res;
 	int err;
 
-	pd = pdev->dev.platform_data;
+	if (pdev->dev.of_node) {
+		struct device_node *np = NULL;
+
+		/* when all users of this driver use FDT, we can remove this */

Ian Molton <[email protected]> writes:
Hi,

[...]

Quoted 13 lines

drivers/net/ethernet/marvell/mv643xx_eth.c +8 -1

@@ -2873,7 +2913,31 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 	struct resource *res;
 	int err;
 
-	pd = pdev->dev.platform_data;
+	if (pdev->dev.of_node) {
+		struct device_node *np = NULL;
+
+		/* when all users of this driver use FDT, we can remove this */
+		pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
+		if (!pd) {
+			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
+			return -ENOMEM;

drivers/net/ethernet/marvell/mv643xx_eth.c static int mv643xx_eth_probe(struct platform_device *pdev) · line 2929

Quoted 6 lines

drivers/net/ethernet/marvell/mv643xx_eth.c +6 -0

+		}
+
+		of_property_read_u32(pdev->dev.of_node,
+			"port_number", &pd->port_number);
+		of_property_read_u32(pdev->dev.of_node,
+			"phy_addr", &pd->phy_addr);

I guess we need something for tx_csum_limit in the device tree too. It's important for kirkwood and dove.

Quoted 18 lines · Arnaud Patard (Rtp)

Ian Molton <[email protected]> writes:
Hi,

[...]

Quoted 13 lines

drivers/net/ethernet/marvell/mv643xx_eth.c +8 -1

@@ -2873,7 +2913,31 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 	struct resource *res;
 	int err;
 
-	pd = pdev->dev.platform_data;
+	if (pdev->dev.of_node) {
+		struct device_node *np = NULL;
+
+		/* when all users of this driver use FDT, we can remove this */
+		pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
+		if (!pd) {
+			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
+			return -ENOMEM;

Quoted 8 lines

drivers/net/ethernet/marvell/mv643xx_eth.c static int mv643xx_eth_probe(struct platform_device *pdev) · line 2929

Quoted 6 lines

drivers/net/ethernet/marvell/mv643xx_eth.c +6 -0

+		}
+
+		of_property_read_u32(pdev->dev.of_node,
+			"port_number", &pd->port_number);
+		of_property_read_u32(pdev->dev.of_node,
+			"phy_addr", &pd->phy_addr);

I guess we need something for tx_csum_limit in the device tree too. It's important for kirkwood and dove.

It was my intention to keep the patch as non-invasive as possible - none of the platforms using D-T at present require this.

Furthermore, git grep shows nothing in mach-kirkwood or mach-dove using at all...

Are you sure this is required? It can always be added if anything actually uses it :)

Quoted 2 lines · Ian Molton

Are you sure this is required? It can always be added if anything actually uses it :)

Hi Ian

Try using jumbo packets on kirkwood.

http://comments.gmane.org/gmane.linux.ports.arm.kernel/178785
http://www.spinics.net/lists/arm-kernel/msg186152.html

Quoted 3 lines · Andrew Lunn

Quoted 2 lines · Ian Molton

Are you sure this is required? It can always be added if anything actually uses it :)

Quoted 6 lines

Hi Ian

Try using jumbo packets on kirkwood.

http://comments.gmane.org/gmane.linux.ports.arm.kernel/178785
http://www.spinics.net/lists/arm-kernel/msg186152.html

Ah, this postdates the D-T branches on which I am working being forked.

I'll fold this into my patchset.

Quoted 1 line

Ah, this postdates the D-T branches on which I am working being forked.

the original patch was from years ago but I noticed only recently it was still not merged and sent this patch. I asked about it on DT because I don't want it to get lost again, even if it looks like not a lot of people are using jumbo frames on kirkwood or dove.

Ian Molton<[email protected]> writes:
Hi,

[...]

Quoted 14 lines

drivers/net/ethernet/marvell/mv643xx_eth.c +9 -1

@@ -2873,7 +2913,31 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
  	struct resource *res;
  	int err;
 
-	pd = pdev->dev.platform_data;
+	if (pdev->dev.of_node) {
+		struct device_node *np = NULL;
+
+		/* when all users of this driver use FDT, we can remove this */
+		pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
+		if (!pd) {
+			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
+			return -ENOMEM;
+		}

drivers/net/ethernet/marvell/mv643xx_eth.c static int mv643xx_eth_probe(struct platform_device *pdev)

Quoted 5 lines

drivers/net/ethernet/marvell/mv643xx_eth.c +5 -0

+
+		of_property_read_u32(pdev->dev.of_node,
+			"port_number",&pd->port_number);
+		of_property_read_u32(pdev->dev.of_node,
+			"phy_addr",&pd->phy_addr);

I guess we need something for tx_csum_limit in the device tree too. It's important for kirkwood and dove.

Quoted 19 lines · Arnaud Patard (Rtp)

Ian Molton<[email protected]> writes:
Hi,

[...]

Quoted 14 lines

drivers/net/ethernet/marvell/mv643xx_eth.c +9 -1

@@ -2873,7 +2913,31 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
  	struct resource *res;
  	int err;
 
-	pd = pdev->dev.platform_data;
+	if (pdev->dev.of_node) {
+		struct device_node *np = NULL;
+
+		/* when all users of this driver use FDT, we can remove this */
+		pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
+		if (!pd) {
+			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
+			return -ENOMEM;
+		}

Quoted 8 lines

drivers/net/ethernet/marvell/mv643xx_eth.c static int mv643xx_eth_probe(struct platform_device *pdev)

Quoted 5 lines

drivers/net/ethernet/marvell/mv643xx_eth.c +5 -0

+
+		of_property_read_u32(pdev->dev.of_node,
+			"port_number",&pd->port_number);
+		of_property_read_u32(pdev->dev.of_node,
+			"phy_addr",&pd->phy_addr);

I guess we need something for tx_csum_limit in the device tree too. It's important for kirkwood and dove.

Is this something for all kirkwood devices? I think the best way of doing this is to give the driver a new device-tree name and having it set this field in the init sequence.

However we could also do with some of these as dt parameters just in case people want to try and alter them for their own systems.

This patch modifies the ethernet setup helper functions so that they can be used /purely/ to set up the clocks.

This is part of ongoing work to enable device tree support in the mv643xx.c ethernet driver, where as yet the kirkwood platform does not have proper clk support.

This should allow a gradual migration to device tree and later to proper clk support, wherupon the helper functions can be removed entirely.

Signed-off-by Ian Molton <[email protected]>

---
 arch/arm/mach-kirkwood/common.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

arch/arm/mach-kirkwood/common.c +4 -2

index c4b64ad..c1776ea 100644
@@ -299,7 +299,8 @@ void __init kirkwood_ehci_init(void)
  ****************************************************************************/
 void __init kirkwood_ge00_init(struct mv643xx_eth_platform_data *eth_data)
 {
-	orion_ge00_init(eth_data,
+	if (eth_data)
+		orion_ge00_init(eth_data,
 			GE00_PHYS_BASE, IRQ_KIRKWOOD_GE00_SUM,
 			IRQ_KIRKWOOD_GE00_ERR);
 	/* The interface forgets the MAC address assigned by u-boot if

@@ -313,7 +314,8 @@ void __init kirkwood_ge00_init(struct mv643xx_eth_platform_data *eth_data)
  ****************************************************************************/
 void __init kirkwood_ge01_init(struct mv643xx_eth_platform_data *eth_data)
 {
-	orion_ge01_init(eth_data,
+	if (eth_data)
+		orion_ge01_init(eth_data,
 			GE01_PHYS_BASE, IRQ_KIRKWOOD_GE01_SUM,
 			IRQ_KIRKWOOD_GE01_ERR);
 	clk_prepare_enable(ge1);

Signature · 1 line

1.7.9.5

Quoted 2 lines · Ian Molton

This patch modifies the ethernet setup helper functions so that they can be used /purely/ to set up the clocks.

Hi Ian

Set up is a bit ambiguous. I would actually say, start the clock ticking, if it is not already.

I'm also not sure this is the best way to do it. I'd like to throw in a counter proposal, and then we can discuss....

Problems start then the ethernet driver is a module, not built in.

I've not looked at the clk DT bindings yet. Does it provide a mechanism to prepare & enable a named clock? Maybe it does, but this seems a bit of an edge case.

What i would instead do is add code to board-dt.c which looks into the DT and see if it finds nodes egige0/egige1 and if so, calls clk_prepare_enable(). We then don't need any per board code.

I find that even if built in, it fails, not because it forgets its MAC, but later, it hangs because as yet it does not know how to handle the clk when it is set up from DT bindings. (Kirkwood doesnt really have this support yet AFAICT).

This patch modifies the ethernet setup helper functions so that they can be used /purely/ to set up the clocks.

This is part of ongoing work to enable device tree support in the mv643xx.c ethernet driver, where as yet the kirkwood platform does not have proper clk support.

This should allow a gradual migration to device tree and later to proper clk support, wherupon the helper functions can be removed entirely.

Signed-off-by Ian Molton <[email protected]>

---
 arch/arm/mach-kirkwood/common.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

arch/arm/mach-kirkwood/common.c +4 -3

index c4b64ad..c1776ea 100644
@@ -299,7 +299,8 @@ void __init kirkwood_ehci_init(void)
  ****************************************************************************/
 void __init kirkwood_ge00_init(struct mv643xx_eth_platform_data *eth_data)
 {
-       orion_ge00_init(eth_data,
+       if (eth_data)
+               orion_ge00_init(eth_data,
                        GE00_PHYS_BASE, IRQ_KIRKWOOD_GE00_SUM,
                        IRQ_KIRKWOOD_GE00_ERR);
        /* The interface forgets the MAC address assigned by u-boot if

@@ -313,7 +314,8 @@ void __init kirkwood_ge00_init(struct mv643xx_eth_platform_data *eth_data)
  ****************************************************************************/
 void __init kirkwood_ge01_init(struct mv643xx_eth_platform_data *eth_data)
 {
-       orion_ge01_init(eth_data,
+       if (eth_data)
+               orion_ge01_init(eth_data,
                        GE01_PHYS_BASE, IRQ_KIRKWOOD_GE01_SUM,
                        IRQ_KIRKWOOD_GE01_ERR);
        clk_prepare_enable(ge1);
--

1.7.9.5

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

If I'm reading this correctly, if kirkwood_ge00_init is called without any data, all that happens is the clock is enabled. If the right data is passed in, the ethernet interface is initialized, and then the clock is enabled.

Patch 6 in your series sets all the existing DT board files to call kirkwood_ge00_init with no data.

Would it make more sense for readability to separate the clock prep and other interface initialization so DT board files don't appear to be calling for a full turn up despite now handling that in the DTS tree? IE instead of converting all the board-XXX.c to call kirkwood_ge00_init(NULL) have them call a new function, kirkwood_ge00_clk_prep(NULL) instead, assuming the clock can't be handled directly via DTS.

If I'm reading this correctly, if kirkwood_ge00_init is called without any data, all that happens is the clock is enabled. If the right data is passed in, the ethernet interface is initialized, and then the clock is enabled.

Patch 6 in your series sets all the existing DT board files to call kirkwood_ge00_init with no data.

I see no reason to keep the calls now that the driver manages to hang onto its clocks.

Quoted 2 lines

I see no reason to keep the calls now that the driver manages to hang onto its clocks.

Hi Ian

Did you test this with it built as a module? That is the real test....

This patch enables mv643xx based ethernet built into the SoM on the csb1724, via flattened device tree.

Signed-off-by Ian Molton <[email protected]>

---
 arch/arm/boot/dts/kirkwood-csb1724.dts |   25 ++++++++++++++++++
 arch/arm/boot/dts/kirkwood.dtsi        |   16 ++++++++++++
 arch/arm/configs/csb1724_defconfig     |   45 ++++++++++++++++++++++++++++++++
 arch/arm/mach-kirkwood/board-csb1724.c |    3 +++
 4 files changed, 89 insertions(+)

arch/arm/boot/dts/kirkwood-csb1724.dts +25 -0

index 44dfe9a..f652797 100644
@@ -25,6 +25,31 @@
 			nr-ports = <2>;
 			status = "ok";
 		};
+
+		smi0: mdio at 72000 {
+			status = "ok";
+		};
+
+		smi1: mdio at 76000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			compatible = "marvell,mv643xx";
+			mdio = <&smi0>;
+			port_number = <0>;
+			phy_addr = <0x80>;
+			interrupts = <11>;
+		};
+
+		egiga1 {
+			compatible = "marvell,mv643xx";
+			mdio = <&smi1>;
+			port_number = <0>;
+			phy_addr = <0x81>;
+			interrupts = <15>;
+		};
+
 	};
 
 };

arch/arm/boot/dts/kirkwood.dtsi +6 -0

index cef9616..dd10422 100644
@@ -105,5 +105,21 @@
 			clock-frequency = <100000>;
 			status = "disabled";
 		};
+
+		smi0: mdio at 72000 {
+			compatible = "marvell,mdio-mv643xx";
+			reg = <0x72000 0x4000>;
+			interrupts = <46>;
+			status = "disabled";

On Mon, Jul 30, 2012 at 04:15:58PM +0100, Ian Molton wrote:

Hi Ian

I'm trying to keep this in order. Currently i2c is in the wrong place, but the rest are incremental. It looks like these should be between wdt & sata.

What about default entries for egiga0 and egiga1 with status "disabled"? It looks like the only thing which changes between boards is phy_addr.

This patch enables mv643xx based ethernet built into the SoM on the csb1724, via flattened device tree.

Signed-off-by Ian Molton <[email protected]>

---
 arch/arm/boot/dts/kirkwood-csb1724.dts |   25 ++++++++++++++++++
 arch/arm/boot/dts/kirkwood.dtsi        |   16 ++++++++++++
 arch/arm/configs/csb1724_defconfig     |   45 ++++++++++++++++++++++++++++++++
 arch/arm/mach-kirkwood/board-csb1724.c |    3 +++
 4 files changed, 89 insertions(+)

arch/arm/boot/dts/kirkwood-csb1724.dts +6 -0

index 44dfe9a..f652797 100644
@@ -25,6 +25,31 @@
 			nr-ports = <2>;
 			status = "ok";
 		};
+
+		smi0: mdio at 72000 {
+			status = "ok";
+		};
+
+		smi1: mdio at 76000 {

Hi Ian

Shouldn't there be two interrupt here?

292 orion_ge00_init(eth_data, 293
GE00_PHYS_BASE, IRQ_KIRKWOOD_GE00_SUM, 294
IRQ_KIRKWOOD_GE00_ERR);