ARM: kirkwood: fdt: convert kirkwood init funcs to fdt

We need to absorb kirkwood_init() into kirkwood_dt_init() so that as we convert drivers, we can remove the platform call, eg kirkwood_rtc_init(). This maintains compatibility with non-fdt configurations because they still call kirkwood_init() in common.c.

As drivers are converted, we will reinstate the 'static' qualifier in common.c.

Signed-off-by Jason Cooper <[email protected]>

---
 arch/arm/mach-kirkwood/board-dt.c |   30 +++++++++++++++++++++++++++++-
 arch/arm/mach-kirkwood/common.c   |   12 ++++++------
 arch/arm/mach-kirkwood/common.h   |    8 ++++++++
 3 files changed, 43 insertions(+), 7 deletions(-)

arch/arm/mach-kirkwood/board-dt.c +2 -0

index 4960e63..a0cc28b 100644
@@ -29,7 +29,9 @@
 #include <linux/spi/orion_spi.h>
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
+#include <asm/mach/map.h>
 #include <mach/kirkwood.h>
+#include <mach/bridge-regs.h>
 #include <plat/mvsdio.h>
 #include "common.h"
 #include "mpp.h"

The comment at mach-kirkwood/common.c:469, says this device is in every kirkwood board. So, it is placed in kirkwood.dtsi.

drivers/rtc/rtc-mv.c +5 -0

@@ -218,10 +220,15 @@ static int __devinit mv_rtc_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	struct rtc_plat_data *pdata;
+	struct device_node *np = pdev->dev.of_node;
 	resource_size_t size;
 	u32 rtc_time;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	if (np)
+		of_address_to_resource(np, 0, res);
+
 	if (!res)
 		return -ENODEV;
 

Quoted 20 lines · Jason Cooper

The comment at mach-kirkwood/common.c:469, says this device is in every kirkwood board. So, it is placed in kirkwood.dtsi.

drivers/rtc/rtc-mv.c +5 -0

@@ -218,10 +220,15 @@ static int __devinit mv_rtc_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	struct rtc_plat_data *pdata;
+	struct device_node *np = pdev->dev.of_node;
 	resource_size_t size;
 	u32 rtc_time;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	if (np)
+		of_address_to_resource(np, 0, res);
+
 	if (!res)
 		return -ENODEV;
 

drivers/rtc/rtc-mv.c static int __devinit mv_rtc_probe(struct platform_device *pdev) · line 271

Quoted 8 lines

drivers/rtc/rtc-mv.c +2 -0

@@ -257,6 +264,8 @@ static int __devinit mv_rtc_probe(struct platform_device *pdev)
 
 	pdata->irq = platform_get_irq(pdev, 0);
 
+	of_property_read_u32(np, "interrupts", &pdata->irq);
+
 	platform_set_drvdata(pdev, pdata);
 
 	if (pdata->irq >= 0) {

The two additions here don't seem necessary, the second one not even
correct: Any interrupts and register properties automatically get turned into platform resources when the device is instatiated.

The interrupt number should not be taken verbatim from the device tree because it is always local to the "interrupt-parent" node, so you would have to use irq_of_parse_and map rather than reading the property directly, but as I said you don't even need to do that for platform devices.

The comment at mach-kirkwood/common.c:469, says this device is in every kirkwood board. So, it is placed in kirkwood.dtsi.

Signed-off-by Jason Cooper <[email protected]>

---
 arch/arm/boot/dts/kirkwood.dtsi   |    6 ++++++
 arch/arm/mach-kirkwood/board-dt.c |    1 -
 arch/arm/mach-kirkwood/common.c   |    2 +-
 arch/arm/mach-kirkwood/common.h   |    1 -
 drivers/rtc/rtc-mv.c              |   19 +++++++++++++++++++
 5 files changed, 26 insertions(+), 3 deletions(-)

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

index 771c6bb..5fb185c 100644
@@ -2,5 +2,11 @@
 
 / {
 	compatible = "marvell,kirkwood";
+
+	rtc at f1010300 {
+		compatible = "marvell,rtc";
+		reg = <0xf1010300 0x1f>;
+		interrupts = <53>;
+	};
 };
 

arch/arm/mach-kirkwood/board-dt.c +0 -1

index a0cc28b..6fc41ae 100644
@@ -135,7 +135,6 @@ static void __init kirkwood_dt_init(void)
 #endif
 
 	/* internal devices that every board has */
-	kirkwood_rtc_init();
 	kirkwood_wdt_init();
 	kirkwood_xor0_init();
 	kirkwood_xor1_init();

arch/arm/mach-kirkwood/common.c +0 -0

index 167b6c8..0c0375f 100644

arch/arm/mach-kirkwood/common.c +0 -0

 {
 	orion_rtc_init(RTC_PHYS_BASE, IRQ_KIRKWOOD_RTC);
 }

arch/arm/mach-kirkwood/common.h +0 -1

index c382447..ca08826 100644
@@ -53,7 +53,6 @@ void kirkwood_restart(char, const char *);
 
 char *kirkwood_id(void);
 void kirkwood_l2_init(void);
-void kirkwood_rtc_init(void);
 void kirkwood_wdt_init(void);
 void kirkwood_xor0_init(void);
 void kirkwood_xor1_init(void);

drivers/rtc/rtc-mv.c +2 -0

index 768e2ed..38abbf7 100644
@@ -12,6 +12,8 @@
 #include <linux/bcd.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
+#include <linux/of_address.h>
+#include <linux/of.h>
 #include <linux/delay.h>
 #include <linux/gfp.h>
 #include <linux/module.h>

@@ -218,10 +220,15 @@ static int __devinit mv_rtc_probe(struct platform_device *pdev)
 {
 	struct resource *res;

drivers/rtc/rtc-mv.c +5 -0

 	platform_set_drvdata(pdev, pdata);
 
 	if (pdata->irq >= 0) {

@@ -294,11 +303,21 @@ static int __exit mv_rtc_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id rtc_mv_of_match_table[] = {
+	{ .compatible = "marvell,rtc", },
+	{}
+};

drivers/rtc/rtc-mv.c +0 -0

 	},
 };
 

Signature · 1 line

1.7.3.4

arch/arm/mach-kirkwood/common.c void __init kirkwood_nand_init_rnb(struct mtd_partition *parts, int nr_parts, · line 166

Quoted 8 lines

arch/arm/mach-kirkwood/common.c +1 -1

@@ -163,7 +163,7 @@ void __init kirkwood_nand_init_rnb(struct mtd_partition *parts, int nr_parts,
 /*****************************************************************************
  * SoC RTC
  ****************************************************************************/
-void __init kirkwood_rtc_init(void)
+static void __init kirkwood_rtc_init(void)

This undoes the code changed in the previous patch. If you reorder the patches then this change goes away in both.

Quoted 45 lines · Jason Cooper

The comment at mach-kirkwood/common.c:469, says this device is in every kirkwood board. So, it is placed in kirkwood.dtsi.

Signed-off-by Jason Cooper <[email protected]>

---
 arch/arm/boot/dts/kirkwood.dtsi   |    6 ++++++
 arch/arm/mach-kirkwood/board-dt.c |    1 -
 arch/arm/mach-kirkwood/common.c   |    2 +-
 arch/arm/mach-kirkwood/common.h   |    1 -
 drivers/rtc/rtc-mv.c              |   19 +++++++++++++++++++
 5 files changed, 26 insertions(+), 3 deletions(-)

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

index 771c6bb..5fb185c 100644
@@ -2,5 +2,11 @@
 
 / {
 	compatible = "marvell,kirkwood";
+
+	rtc at f1010300 {
+		compatible = "marvell,rtc";
+		reg = <0xf1010300 0x1f>;
+		interrupts = <53>;
+	};
 };
 

arch/arm/mach-kirkwood/board-dt.c +0 -1

index a0cc28b..6fc41ae 100644
@@ -135,7 +135,6 @@ static void __init kirkwood_dt_init(void)
 #endif
 
 	/* internal devices that every board has */
-	kirkwood_rtc_init();
 	kirkwood_wdt_init();
 	kirkwood_xor0_init();
 	kirkwood_xor1_init();

arch/arm/mach-kirkwood/common.c +0 -0

index 167b6c8..0c0375f 100644
@@ -163,7 +163,7 @@ void __init kirkwood_nand_init_rnb(struct mtd_partition *parts, int nr_parts,

arch/arm/mach-kirkwood/common.c void __init kirkwood_nand_init_rnb(struct mtd_partition *parts, int nr_parts, · line 166

Quoted 5 lines

arch/arm/mach-kirkwood/common.c +1 -1

 /*****************************************************************************
  * SoC RTC
  ****************************************************************************/
-void __init kirkwood_rtc_init(void)
+static void __init kirkwood_rtc_init(void)

This undoes the code changed in the previous patch. If you reorder the patches then this change goes away in both.

Quoted 36 lines

arch/arm/mach-kirkwood/common.c +0 -0

 {
 	orion_rtc_init(RTC_PHYS_BASE, IRQ_KIRKWOOD_RTC);
 }

arch/arm/mach-kirkwood/common.h +0 -1

index c382447..ca08826 100644
@@ -53,7 +53,6 @@ void kirkwood_restart(char, const char *);
 
 char *kirkwood_id(void);
 void kirkwood_l2_init(void);
-void kirkwood_rtc_init(void);
 void kirkwood_wdt_init(void);
 void kirkwood_xor0_init(void);
 void kirkwood_xor1_init(void);

drivers/rtc/rtc-mv.c +3 -0

index 768e2ed..38abbf7 100644
@@ -12,6 +12,8 @@
 #include <linux/bcd.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
+#include <linux/of_address.h>
+#include <linux/of.h>
 #include <linux/delay.h>
 #include <linux/gfp.h>
 #include <linux/module.h>

@@ -218,10 +220,15 @@ static int __devinit mv_rtc_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	struct rtc_plat_data *pdata;
+	struct device_node *np = pdev->dev.of_node;
 	resource_size_t size;
 	u32 rtc_time;
 

drivers/rtc/rtc-mv.c static int __devinit mv_rtc_probe(struct platform_device *pdev) · line 231

Quoted 5 lines

drivers/rtc/rtc-mv.c +4 -0

 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	if (np)
+		of_address_to_resource(np, 0, res);
+

This should not be needed. The of_platform_populate() code should have filled the platform_device with mem resources.

Quoted 5 lines

drivers/rtc/rtc-mv.c +0 -0

 	if (!res)
 		return -ENODEV;
 

@@ -257,6 +264,8 @@ static int __devinit mv_rtc_probe(struct platform_device *pdev)
 

drivers/rtc/rtc-mv.c static int __devinit mv_rtc_probe(struct platform_device *pdev) · line 268

Quoted 4 lines

drivers/rtc/rtc-mv.c +2 -0

 	pdata->irq = platform_get_irq(pdev, 0);
 
+	of_property_read_u32(np, "interrupts", &pdata->irq);
+

Definitely don't do this. Same as with memory regions, the irq will be pre-populated in the platform device resource table. platform_get_irq() should already work.

Quoted 12 lines

drivers/rtc/rtc-mv.c +5 -0

 	platform_set_drvdata(pdev, pdata);
 
 	if (pdata->irq >= 0) {

@@ -294,11 +303,21 @@ static int __exit mv_rtc_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id rtc_mv_of_match_table[] = {
+	{ .compatible = "marvell,rtc", },
+	{}
+};

drivers/rtc/rtc-mv.c static int __exit mv_rtc_remove(struct platform_device *pdev) · line 320

Quoted 10 lines

drivers/rtc/rtc-mv.c +5 -0

+#else
+#define rtc_mv_of_match_table NULL
+#endif
+
 static struct platform_driver mv_rtc_driver = {
 	.remove		= __exit_p(mv_rtc_remove),
 	.driver		= {
 		.name	= "rtc-mv",
 		.owner	= THIS_MODULE,
+		.of_match_table = rtc_mv_of_match_table,

should be:
.of_match_table = of_match_ptr(rtc_mv_of_match_table),

that would get rid of the #else clause above.

The comment at mach-kirkwood/common.c:469, says this device is in every kirkwood board. So, it is placed in kirkwood.dtsi.

Signed-off-by Jason Cooper <[email protected]>

---
 arch/arm/boot/dts/kirkwood.dtsi   |    6 ++++++
 arch/arm/mach-kirkwood/board-dt.c |    1 -
 arch/arm/mach-kirkwood/common.c   |    2 +-
 arch/arm/mach-kirkwood/common.h   |    1 -
 drivers/rtc/rtc-mv.c              |   19 +++++++++++++++++++
 5 files changed, 26 insertions(+), 3 deletions(-)

(quoted diff) +0 -0

b/arch/arm/boot/dts/kirkwood.dtsi index 771c6bb..5fb185c 100644

in my older patch series i flagged all converted devices with two compatibility names. Eg:

{ .compatible = "marvell,orion5x-rtc", },
{ .compatible = "marvell,kirkwood-rtc", },

IMHO marvell,rtc is i little too general, i guess marvell won't have only one rtc ;)

Other opinions?

Quoted 9 lines · Michael Walle

in my older patch series i flagged all converted devices with two compatibility names. Eg:

{ .compatible = "marvell,orion5x-rtc", },
{ .compatible = "marvell,kirkwood-rtc", },

IMHO marvell,rtc is i little too general, i guess marvell won't have only one rtc ;)

Quoted 1 line

Other opinions?

Being specific is certainly good, and it may also be a good idea to list the first one it's compatible with in the device tree source.

So the device tree for a 88F6281 soc could list

	compatible = "marvell,88f6281-rtc", "marvell,kirkwood-rtc", "marvell,orion-rtc";

This would give the device driver the option which one to bind to. Anything that has an rtc compatible with the one in orion should list that one, so that the driver only has to list that one if they are truely identical, but if the driver has to get modified later to know the difference between orion and kirkwood rtcs, it can check the more specific value.

Arnd

Quoted 17 lines · Michael Walle

Quoted 10 lines

in my older patch series i flagged all converted devices with two

compatibility names. Eg:
{ .compatible = "marvell,orion5x-rtc", },
{ .compatible = "marvell,kirkwood-rtc", },

IMHO marvell,rtc is i little too general, i guess marvell won't have only one rtc ;)

Other opinions?

Being specific is certainly good, and it may also be a good idea to list the first one it's compatible with in the device tree source.

So the device tree for a 88F6281 soc could list

Quoted 2 lines

	compatible = "marvell,88f6281-rtc", "marvell,kirkwood-rtc",
	"marvell,orion-rtc";

I'll go ahead and make this style of change throughout. I assume "marvell,88f6281-rtc" would only be for SoC internal devices? eg, the spi driver wouldn't follow that convention. I'll see if I can dig up a part number on it.

static int heartbeat = -1; /* module parameter (seconds) */
static unsigned int wdt_max_duration; /* (seconds) */ static unsigned int wdt_tclk;
+static unsigned int wdt_reg;
static unsigned long wdt_status;
static DEFINE_SPINLOCK(wdt_lock);

Converted register address to a variable so that it could be set from fdt. Also, pull clock frequency from fdt and clean up failure logic a bit.

Same as rtc-mv, this device is used in all kirkwood boards. So, it is placed in kirkwood.dtsi.

Signed-off-by Jason Cooper <[email protected]>

---
 arch/arm/boot/dts/kirkwood.dtsi   |    6 +++++
 arch/arm/mach-kirkwood/board-dt.c |    1 -
 arch/arm/mach-kirkwood/common.c   |    2 +-
 arch/arm/mach-kirkwood/common.h   |    1 -
 drivers/watchdog/orion_wdt.c      |   44 ++++++++++++++++++++++++++----------
 5 files changed, 39 insertions(+), 15 deletions(-)

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

index 5fb185c..bf28424 100644
@@ -8,5 +8,11 @@
 		reg = <0xf1010300 0x1f>;
 		interrupts = <53>;
 	};
+
+	wdt at fed20300 {
+		compatible = "marvell,orion-wdt";
+		reg = <0xfed20300 0x28>;
+		clock-frequency = <200000000>;
+	};
 };
 

arch/arm/mach-kirkwood/board-dt.c +0 -1

index 6fc41ae..7ef5fb7 100644
@@ -135,7 +135,6 @@ static void __init kirkwood_dt_init(void)
 #endif
 
 	/* internal devices that every board has */
-	kirkwood_wdt_init();
 	kirkwood_xor0_init();
 	kirkwood_xor1_init();
 	kirkwood_crypto_init();

arch/arm/mach-kirkwood/common.c +1 -1

index 0c0375f..35dc40b 100644
@@ -303,7 +303,7 @@ void __init kirkwood_xor1_init(void)
 /*****************************************************************************
  * Watchdog
  ****************************************************************************/
-void __init kirkwood_wdt_init(void)
+static void __init kirkwood_wdt_init(void)
 {
 	orion_wdt_init(kirkwood_tclk);
 }

arch/arm/mach-kirkwood/common.h +0 -0

index ca08826..ac4b2fb 100644

arch/arm/mach-kirkwood/common.h +0 -0

 void kirkwood_xor0_init(void);
 void kirkwood_xor1_init(void);
 void kirkwood_crypto_init(void);

drivers/watchdog/orion_wdt.c +15 -12

index 4ad78f8..4be76ca 100644
@@ -17,6 +17,7 @@
 #include <linux/fs.h>
 #include <linux/miscdevice.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
 #include <linux/watchdog.h>
 #include <linux/init.h>
 #include <linux/uaccess.h>

@@ -28,9 +29,9 @@
 /*
  * Watchdog timer block registers.
  */
-#define TIMER_CTRL		(TIMER_VIRT_BASE + 0x0000)
+#define TIMER_CTRL		0x0000
 #define  WDT_EN			0x0010
-#define WDT_VAL			(TIMER_VIRT_BASE + 0x0024)
+#define WDT_VAL			0x0024
 
 #define WDT_MAX_CYCLE_COUNT	0xffffffff
 #define WDT_IN_USE		0

@@ -40,6 +41,7 @@ static int nowayout = WATCHDOG_NOWAYOUT;
 static int heartbeat = -1;		/* module parameter (seconds) */
 static unsigned int wdt_max_duration;	/* (seconds) */
 static unsigned int wdt_tclk;
+static unsigned int wdt_reg;
 static unsigned long wdt_status;
 static DEFINE_SPINLOCK(wdt_lock);
 

@@ -48,7 +50,7 @@ static void orion_wdt_ping(void)
 	spin_lock(&wdt_lock);
 
 	/* Reload watchdog duration */
-	writel(wdt_tclk * heartbeat, WDT_VAL);
+	writel(wdt_tclk * heartbeat, wdt_reg + WDT_VAL);
 
 	spin_unlock(&wdt_lock);
 }

@@ -60,7 +62,7 @@ static void orion_wdt_enable(void)
 	spin_lock(&wdt_lock);
 
 	/* Set watchdog duration */
-	writel(wdt_tclk * heartbeat, WDT_VAL);
+	writel(wdt_tclk * heartbeat, wdt_reg + WDT_VAL);
 
 	/* Clear watchdog timer interrupt */
 	reg = readl(BRIDGE_CAUSE);

@@ -68,9 +70,9 @@ static void orion_wdt_enable(void)
 	writel(reg, BRIDGE_CAUSE);
 
 	/* Enable watchdog timer */
-	reg = readl(TIMER_CTRL);
+	reg = readl(wdt_reg + TIMER_CTRL);
 	reg |= WDT_EN;
-	writel(reg, TIMER_CTRL);
+	writel(reg, wdt_reg + TIMER_CTRL);
 
 	/* Enable reset on watchdog */
 	reg = readl(RSTOUTn_MASK);

@@ -92,9 +94,9 @@ static void orion_wdt_disable(void)
 	writel(reg, RSTOUTn_MASK);
 
 	/* Disable watchdog timer */
-	reg = readl(TIMER_CTRL);
+	reg = readl(wdt_reg + TIMER_CTRL);
 	reg &= ~WDT_EN;
-	writel(reg, TIMER_CTRL);
+	writel(reg, wdt_reg + TIMER_CTRL);
 
 	spin_unlock(&wdt_lock);
 }

@@ -102,7 +104,7 @@ static void orion_wdt_disable(void)
 static int orion_wdt_get_timeleft(int *time_left)
 {
 	spin_lock(&wdt_lock);
-	*time_left = readl(WDT_VAL) / wdt_tclk;
+	*time_left = readl(wdt_reg + WDT_VAL) / wdt_tclk;
 	spin_unlock(&wdt_lock);
 	return 0;
 }

@@ -236,15 +238,23 @@ static struct miscdevice orion_wdt_miscdev = {
 static int __devinit orion_wdt_probe(struct platform_device *pdev)
 {
 	struct orion_wdt_platform_data *pdata = pdev->dev.platform_data;
+	struct device_node *np = pdev->dev.of_node;
 	int ret;
 
-	if (pdata) {
+	if (pdata)
 		wdt_tclk = pdata->tclk;
-	} else {
-		printk(KERN_ERR "Orion Watchdog misses platform data\n");
+
+	of_property_read_u32(np, "clock-frequency", &wdt_tclk);

drivers/watchdog/orion_wdt.c +9 -0

 	if (orion_wdt_miscdev.parent)
 		return -EBUSY;
 	orion_wdt_miscdev.parent = &pdev->dev;

@@ -284,6 +294,15 @@ static void orion_wdt_shutdown(struct platform_device *pdev)
 		orion_wdt_disable();
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id orion_wdt_of_match_table[] = {
+	{ .compatible = "marvell,orion-wdt", },
+	{},
+};
+#else
+#define orion_wdt_of_match_table NULL
+#endif
+
 static struct platform_driver orion_wdt_driver = {
 	.probe		= orion_wdt_probe,
 	.remove		= __devexit_p(orion_wdt_remove),

drivers/watchdog/orion_wdt.c +0 -0

 	},
 };
 

Signature · 1 line

1.7.3.4

Converted register address to a variable so that it could be set from fdt. Also, pull clock frequency from fdt and clean up failure logic a bit.

Same as rtc-mv, this device is used in all kirkwood boards. So, it is placed in kirkwood.dtsi.

Signed-off-by Jason Cooper <[email protected]>

---
 arch/arm/boot/dts/kirkwood.dtsi   |    6 +++++
 arch/arm/mach-kirkwood/board-dt.c |    1 -
 arch/arm/mach-kirkwood/common.c   |    2 +-
 arch/arm/mach-kirkwood/common.h   |    1 -
 drivers/watchdog/orion_wdt.c      |   44 ++++++++++++++++++++++++++----------
 5 files changed, 39 insertions(+), 15 deletions(-)

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

index 5fb185c..bf28424 100644

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

 };
 

arch/arm/mach-kirkwood/board-dt.c +0 -1

index 6fc41ae..7ef5fb7 100644
@@ -135,7 +135,6 @@ static void __init kirkwood_dt_init(void)
 #endif
 
 	/* internal devices that every board has */
-	kirkwood_wdt_init();
 	kirkwood_xor0_init();
 	kirkwood_xor1_init();
 	kirkwood_crypto_init();

arch/arm/mach-kirkwood/common.c +1 -1

index 0c0375f..35dc40b 100644
@@ -303,7 +303,7 @@ void __init kirkwood_xor1_init(void)
 /*****************************************************************************
  * Watchdog
  ****************************************************************************/
-void __init kirkwood_wdt_init(void)
+static void __init kirkwood_wdt_init(void)
 {
 	orion_wdt_init(kirkwood_tclk);
 }

arch/arm/mach-kirkwood/common.h +0 -1

index ca08826..ac4b2fb 100644
@@ -53,7 +53,6 @@ void kirkwood_restart(char, const char *);
 
 char *kirkwood_id(void);
 void kirkwood_l2_init(void);
-void kirkwood_wdt_init(void);
 void kirkwood_xor0_init(void);
 void kirkwood_xor1_init(void);
 void kirkwood_crypto_init(void);

drivers/watchdog/orion_wdt.c +32 -12

index 4ad78f8..4be76ca 100644
@@ -17,6 +17,7 @@
 #include <linux/fs.h>
 #include <linux/miscdevice.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
 #include <linux/watchdog.h>
 #include <linux/init.h>
 #include <linux/uaccess.h>

@@ -28,9 +29,9 @@
 /*
  * Watchdog timer block registers.
  */
-#define TIMER_CTRL		(TIMER_VIRT_BASE + 0x0000)
+#define TIMER_CTRL		0x0000
 #define  WDT_EN			0x0010
-#define WDT_VAL			(TIMER_VIRT_BASE + 0x0024)
+#define WDT_VAL			0x0024
 
 #define WDT_MAX_CYCLE_COUNT	0xffffffff
 #define WDT_IN_USE		0

@@ -40,6 +41,7 @@ static int nowayout = WATCHDOG_NOWAYOUT;
 static int heartbeat = -1;		/* module parameter (seconds) */
 static unsigned int wdt_max_duration;	/* (seconds) */
 static unsigned int wdt_tclk;
+static unsigned int wdt_reg;
 static unsigned long wdt_status;
 static DEFINE_SPINLOCK(wdt_lock);
 

@@ -48,7 +50,7 @@ static void orion_wdt_ping(void)
 	spin_lock(&wdt_lock);
 
 	/* Reload watchdog duration */
-	writel(wdt_tclk * heartbeat, WDT_VAL);
+	writel(wdt_tclk * heartbeat, wdt_reg + WDT_VAL);
 
 	spin_unlock(&wdt_lock);
 }

@@ -60,7 +62,7 @@ static void orion_wdt_enable(void)
 	spin_lock(&wdt_lock);
 
 	/* Set watchdog duration */
-	writel(wdt_tclk * heartbeat, WDT_VAL);
+	writel(wdt_tclk * heartbeat, wdt_reg + WDT_VAL);
 
 	/* Clear watchdog timer interrupt */
 	reg = readl(BRIDGE_CAUSE);

@@ -68,9 +70,9 @@ static void orion_wdt_enable(void)
 	writel(reg, BRIDGE_CAUSE);
 
 	/* Enable watchdog timer */
-	reg = readl(TIMER_CTRL);
+	reg = readl(wdt_reg + TIMER_CTRL);
 	reg |= WDT_EN;
-	writel(reg, TIMER_CTRL);
+	writel(reg, wdt_reg + TIMER_CTRL);
 
 	/* Enable reset on watchdog */
 	reg = readl(RSTOUTn_MASK);

@@ -92,9 +94,9 @@ static void orion_wdt_disable(void)
 	writel(reg, RSTOUTn_MASK);
 
 	/* Disable watchdog timer */
-	reg = readl(TIMER_CTRL);
+	reg = readl(wdt_reg + TIMER_CTRL);
 	reg &= ~WDT_EN;
-	writel(reg, TIMER_CTRL);
+	writel(reg, wdt_reg + TIMER_CTRL);
 
 	spin_unlock(&wdt_lock);
 }

@@ -102,7 +104,7 @@ static void orion_wdt_disable(void)
 static int orion_wdt_get_timeleft(int *time_left)
 {
 	spin_lock(&wdt_lock);
-	*time_left = readl(WDT_VAL) / wdt_tclk;
+	*time_left = readl(wdt_reg + WDT_VAL) / wdt_tclk;
 	spin_unlock(&wdt_lock);
 	return 0;
 }

@@ -236,15 +238,23 @@ static struct miscdevice orion_wdt_miscdev = {
 static int __devinit orion_wdt_probe(struct platform_device *pdev)
 {
 	struct orion_wdt_platform_data *pdata = pdev->dev.platform_data;
+	struct device_node *np = pdev->dev.of_node;
 	int ret;
 
-	if (pdata) {
+	if (pdata)
 		wdt_tclk = pdata->tclk;
-	} else {
-		printk(KERN_ERR "Orion Watchdog misses platform data\n");
+
+	of_property_read_u32(np, "clock-frequency", &wdt_tclk);
+
+	if (!wdt_tclk) {
+		printk(KERN_ERR "Orion Watchdog can't get clock freq\n");
 		return -ENODEV;
 	}
 
+	wdt_reg = TIMER_VIRT_BASE;
+
+	of_property_read_u32(np, "reg", &wdt_reg);
+
 	if (orion_wdt_miscdev.parent)
 		return -EBUSY;
 	orion_wdt_miscdev.parent = &pdev->dev;

@@ -284,6 +294,15 @@ static void orion_wdt_shutdown(struct platform_device *pdev)
 		orion_wdt_disable();
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id orion_wdt_of_match_table[] = {
+	{ .compatible = "marvell,orion-wdt", },
+	{},
+};
+#else
+#define orion_wdt_of_match_table NULL
+#endif
+
 static struct platform_driver orion_wdt_driver = {
 	.probe		= orion_wdt_probe,
 	.remove		= __devexit_p(orion_wdt_remove),

@@ -291,6 +310,7 @@ static struct platform_driver orion_wdt_driver = {
 	.driver		= {
 		.owner	= THIS_MODULE,
 		.name	= "orion_wdt",
+		.of_match_table = orion_wdt_of_match_table,
 	},
 };
 

Signature · 7 lines

1.7.3.4


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

Hi Jason,

Quoted 21 lines · Jason Cooper

Converted register address to a variable so that it could be set from fdt. Also, pull clock frequency from fdt and clean up failure logic a bit.

Same as rtc-mv, this device is used in all kirkwood boards. So, it is placed in kirkwood.dtsi.

Signed-off-by Jason Cooper <[email protected]>

---
 arch/arm/boot/dts/kirkwood.dtsi   |    6 +++++
 arch/arm/mach-kirkwood/board-dt.c |    1 -
 arch/arm/mach-kirkwood/common.c   |    2 +-
 arch/arm/mach-kirkwood/common.h   |    1 -
 drivers/watchdog/orion_wdt.c      |   44 ++++++++++++++++++++++++++----------
 5 files changed, 39 insertions(+), 15 deletions(-)

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

index 5fb185c..bf28424 100644

arch/arm/boot/dts/kirkwood.dtsi line 16

Quoted 9 lines

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

@@ -8,5 +8,11 @@
 		reg = <0xf1010300 0x1f>;
 		interrupts = <53>;
 	};
+
+	wdt at fed20300 {
+		compatible = "marvell,orion-wdt";
+		reg = <0xfed20300 0x28>;
+		clock-frequency = <200000000>;
+	};

How this will work for boards using a different clock frequency (TCLK) ? Here, we could have 166MHz for example.

Can this node be updated during the board initialization with a detected TCLK value ?

Or maybe this clock-frequency can reference a TCLK frequency defined in a board specific dts (I don't know if the DT format allow such things) ?

Grr... good catch. I originally had this in kirkwood-dreamplug.dts, which is always 200000000. That's not the case in kirkwood.dtsi. What I would like to do, and I haven't had time to look into it (I thought I would tackle it later as a refinement :-( ), is a reference of some sort, eg:

in kirkwood.dtsi:

/ {
        compatible = "marvell,kirkwood";
        tclk:clock-frequency = <200000000>;

        wdt at fed20300 {
                compatible = "marvell,orion-wdt";
                reg = <0xfed20300 0x28>;
                clock-frequency = &tclk;
        };
};

then, in kirkwood-foobar.dts

include "kirkwood.dtsi"

/ {
        model = "foobar";
        compatible = "...";
        tclk:clock-frequency = <166000000>;
};

but I'm not sure if that would work.

In any case, the simplest answer is to set clock-frequency in kirkwood-dreamplug.dts as a root node property, and then each driver that needs tclk, requests the clock-frequency from the root node. Hopefully, Grant can chime in on this one.

Quoted 29 lines · Jason

Grr... good catch. I originally had this in kirkwood-dreamplug.dts, which is always 200000000. That's not the case in kirkwood.dtsi. What I would like to do, and I haven't had time to look into it (I thought I would tackle it later as a refinement :-( ), is a reference of some sort, eg:

in kirkwood.dtsi:

/ {
        compatible = "marvell,kirkwood";
        tclk:clock-frequency = <200000000>;

        wdt at fed20300 {
                compatible = "marvell,orion-wdt";
                reg = <0xfed20300 0x28>;
                clock-frequency = &tclk;
        };
};

then, in kirkwood-foobar.dts

include "kirkwood.dtsi"

/ {
        model = "foobar";
        compatible = "...";
        tclk:clock-frequency = <166000000>;
};

Quoted 1 line

but I'm not sure if that would work.

That would make wdt at fed20300/clock-frequency a phandle pointing to the root property, which is not what we want here.

Quoted 4 lines

In any case, the simplest answer is to set clock-frequency in kirkwood-dreamplug.dts as a root node property, and then each driver that needs tclk, requests the clock-frequency from the root node. Hopefully, Grant can chime in on this one.

I think you can just pick a reasonable default value for wdt at fed20300/clock-frequency, and let the board override that by setting it to something else if necessary.

I suppose this will also change a bit when kirkwood gets moved over to generic clk support in the future and starts using the clk binding instead of what you do now.

Quoted 3 lines

That would make wdt at fed20300/clock-frequency a phandle pointing to the root property, which is not what we want here.

The 200000000 value is not the actual value used in most drivers [1], but rather the speed of the system clock [2] (hence, my inclination to make it a root property), the drivers have been pulling this number from a global variable [3] and dividing/rounding it as needed for their own needs [1].

Since it's derived from the SoC core [2], it would seem to make sense to have a root "clock-frequency" in the board dts. Am I missing something? Is there a better way to do this?

Quoted 8 lines

Quoted 4 lines

In any case, the simplest answer is to set clock-frequency in kirkwood-dreamplug.dts as a root node property, and then each driver that needs tclk, requests the clock-frequency from the root node. Hopefully, Grant can chime in on this one.

I think you can just pick a reasonable default value for wdt at fed20300/clock-frequency, and let the board override that by setting it to something else if necessary.

True, for now, I can just set clock-frequency for each device to the exact same value and we'll polish later once we have a better idea of the pattern it's following.

Quoted 3 lines

I suppose this will also change a bit when kirkwood gets moved over to generic clk support in the future and starts using the clk binding instead of what you do now.

Yes, I don't want to over-think it, but I would like to make sure it's in the correct place, since it is a global property of the board.

Quoted 9 lines · Jason

Quoted 3 lines

That would make wdt at fed20300/clock-frequency a phandle pointing to the root property, which is not what we want here.

The 200000000 value is not the actual value used in most drivers [1], but rather the speed of the system clock [2] (hence, my inclination to make it a root property), the drivers have been pulling this number from a global variable [3] and dividing/rounding it as needed for their own needs [1].

Ah, I see. If you put a "clock-frequency" property into a device node, it should definitely be the frequency used by that device, not the system clock that it is derived from.

Quoted 3 lines

Since it's derived from the SoC core [2], it would seem to make sense to have a root "clock-frequency" in the board dts. Am I missing something? Is there a better way to do this?

The correct solution would be to use the clock binding, but I'm not sure how far we are in making that final or usable.

Quoted 17 lines

Quoted 8 lines

Quoted 4 lines

In any case, the simplest answer is to set clock-frequency in kirkwood-dreamplug.dts as a root node property, and then each driver that needs tclk, requests the clock-frequency from the root node. Hopefully, Grant can chime in on this one.

I think you can just pick a reasonable default value for wdt at fed20300/clock-frequency, and let the board override that by setting it to something else if necessary.

True, for now, I can just set clock-frequency for each device to the exact same value and we'll polish later once we have a better idea of the pattern it's following.

Quoted 3 lines

I suppose this will also change a bit when kirkwood gets moved over to generic clk support in the future and starts using the clk binding instead of what you do now.

Quoted 2 lines

Yes, I don't want to over-think it, but I would like to make sure it's in the correct place, since it is a global property of the board.

Andrew Lunn said that he has a patch for plat-orion to use the generic clock framework. I think we should look at that before introducing a hack that will have to be removed again very soon.

Kirkwood/orion is probably not a bad platform to add to the initial set of platforms for which we use the common clk infrastructure, so maybe the best way forward for you is to take Andrews patches and build on top of that.

It might mean that your follow-on patches get delayed until v3.5 instead of going into v3.4, but the initial code you have in arm-soc is working already and you can work on getting it the clock code right in the initial code.

Quoted 3 lines

Andrew Lunn said that he has a patch for plat-orion to use the generic clock framework. I think we should look at that before introducing a hack that will have to be removed again very soon.

I asked him for it earlier [1], apparently it's a very invasive patch series which breaks things. I would like to look at it as a reference, even if applying it isn't a good idea.

Quoted 7 lines

Quoted 3 lines

Andrew Lunn said that he has a patch for plat-orion to use the generic clock framework. I think we should look at that before introducing a hack that will have to be removed again very soon.

I asked him for it earlier [1], apparently it's a very invasive patch series which breaks things. I would like to look at it as a reference, even if applying it isn't a good idea.

I did not say it breaks things. I said i have a habit of breaking orion5x, because i cannot test it.

I will send the patches as RFC on Sunday. I'm traveling at the moment.

This series begins the process of converting all of the drivers initialized from kirkwood_init() to devicetree.

The first patch cleans up kirkwood_init() by moving a flag setting to it's appropriate location. This will help later when converting the audio driver.

Next, I pull the functionality of kirkwood_init() into kirkwood_dt_init() in board-dt.c This way, as drivers are converted, their init calls can be removed from kirkwood_dt_init() and non-fdt users won't be affected (they call kirkwood_init()).

Last, I converted the two easiest drivers, rtc and wdt. The xor and crypto drivers remain for a later patch series.

Quoted 6 lines · Jason Cooper

This series begins the process of converting all of the drivers initialized from kirkwood_init() to devicetree.

The first patch cleans up kirkwood_init() by moving a flag setting to it's appropriate location. This will help later when converting the audio driver.

Quoted 7 lines

Next, I pull the functionality of kirkwood_init() into kirkwood_dt_init() in board-dt.c This way, as drivers are converted, their init calls can be removed from kirkwood_dt_init() and non-fdt users won't be affected (they call kirkwood_init()).

Last, I converted the two easiest drivers, rtc and wdt. The xor and crypto drivers remain for a later patch series.

Hi Jason,

Nice patch series! I've commented on a few details that need improving, but it seems you're making good progress overall. Note that in other platforms, we typically focus on converting all the devices outside of the soc first, because that lets us get away from board files faster, but there is nothing wrong in principle in converting the devices on the soc as well.

Arnd

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

Thanks for the feedback. I'll redo this series and resubmit. Did you catch my patch from yesterday?

[PATCH v3] ARM: kirkwood: covert orion-spi to fdt. [1]

Quoted 4 lines · Jason

Thanks for the feedback. I'll redo this series and resubmit. Did you catch my patch from yesterday?

[PATCH v3] ARM: kirkwood: covert orion-spi to fdt. [1]

Yes, I saw that. It looks good and I was just waiting if other people have comments on it. Maybe you can put it into the same git tree as these ones when they are ready and you send a pull request.

Quoted 9 lines · Arnd Bergmann

Quoted 4 lines · Jason

Thanks for the feedback. I'll redo this series and resubmit. Did you catch my patch from yesterday?

[PATCH v3] ARM: kirkwood: covert orion-spi to fdt. [1]

Yes, I saw that. It looks good and I was just waiting if other people have comments on it. Maybe you can put it into the same git tree as these ones when they are ready and you send a pull request.

Okay, I have a local branch, 'dreamplug_dt' based at v3.3-rc3. It includes the two patches you've already merged into your tree:

c9780e7 ARM: kirkwood: add dreamplug (fdt) support. dbc3c26 ARM: kirkwood: convert uart0 to devicetree.

Plus the spi/flash/partitions, and the most recent series.

Can I just do:

git push linux-kirkwood dreamplug_dt:kirkwood_dt

Then, issue a pull request for

git://git.infradead.org/users/jcooper/linux-kirkwood.git kirkwood_dt

and reference the first uncommitted patch?

Most of the googlings for 'pull request' yield github's "click the button", yuck.

Also, is there an equivalent to 'git format-patch/send-email' for pull requests? The pull requests I've looked at seem awfully consistent in formatting.

Quoted 25 lines · Jason

Quoted 9 lines · Arnd Bergmann

Quoted 4 lines · Jason

Thanks for the feedback. I'll redo this series and resubmit. Did you catch my patch from yesterday?

[PATCH v3] ARM: kirkwood: covert orion-spi to fdt. [1]

Yes, I saw that. It looks good and I was just waiting if other people have comments on it. Maybe you can put it into the same git tree as these ones when they are ready and you send a pull request.

Okay, I have a local branch, 'dreamplug_dt' based at v3.3-rc3. It includes the two patches you've already merged into your tree:

c9780e7 ARM: kirkwood: add dreamplug (fdt) support. dbc3c26 ARM: kirkwood: convert uart0 to devicetree.

Plus the spi/flash/partitions, and the most recent series.

Can I just do:

git push linux-kirkwood dreamplug_dt:kirkwood_dt

Then, issue a pull request for

Quoted 3 lines

git://git.infradead.org/users/jcooper/linux-kirkwood.git kirkwood_dt

and reference the first uncommitted patch?

I have other commit IDs for the two patches that I took from your emails, so you should rebase the other patches on top of the version that I have in the arm-soc/kirkwood/board branch, otherwise we end up with duplicate commits my tree when I pull them again.

Quoted 6 lines

Most of the googlings for 'pull request' yield github's "click the button", yuck.

Also, is there an equivalent to 'git format-patch/send-email' for pull requests? The pull requests I've looked at seem awfully consistent in formatting.

It's "git request-pull".

Quoted 5 lines

I have other commit IDs for the two patches that I took from your emails, so you should rebase the other patches on top of the version that I have in the arm-soc/kirkwood/board branch, otherwise we end up with duplicate commits my tree when I pull them again.

Got it, I cherry-picked the series over and incorporated Grant's suggestions.

Quoted 9 lines · Jason

Quoted 5 lines

I have other commit IDs for the two patches that I took from your emails, so you should rebase the other patches on top of the version that I have in the arm-soc/kirkwood/board branch, otherwise we end up with duplicate commits my tree when I pull them again.

Got it, I cherry-picked the series over and incorporated Grant's suggestions.

Just making sure you know about this one:

git rebase -i --onto arm-soc/kirkwood/board torvalds/master

The '-i' lets you pick which ones you want to move over, then it applies all patches from the current branch that are not on torvalds/master on top of the last commit from arm-soc/kirkwood/board.

If you don't know it yet, I suggest you play with this for an hour or so that will be very well invested time. If you knew this, then nevermind.

On the Globalscale Dreamplug (Marvell Kirkwood Development Platform), 2MB of NOR flash are used to hold the bootloader, bootloader environment, and devicetree blob. It is connected via spi.

Signed-off-by Jason Cooper <[email protected]>

---

Changes from v1:

- used of_match_ptr() as suggested by Grant Likely

 arch/arm/boot/dts/kirkwood-dreamplug.dts |   34 +++++++++++++++++++++++++
 arch/arm/mach-kirkwood/board-dt.c        |   40
------------------------------ arch/arm/mach-kirkwood/common.c          | 
 11 ++++++++
 drivers/spi/spi-orion.c                  |   37

++++++++++++++++++++++++--- 4 files changed, 78 insertions(+), 44 deletions(-)

(quoted diff) +0 -0

b/arch/arm/boot/dts/kirkwood-dreamplug.dts index 8a5dff8..bdf2ddc 100644

arch/arm/boot/dts/kirkwood-dreamplug.dts +34 -0

@@ -22,4 +22,38 @@
 		interrupts = <33>;
 		clock-frequency = <200000000>;
 	};
+
+	spi at f1010600 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		compatible = "marvell,orion-spi";
+		reg = <0xf1010600 0x1ff>;
+		clock-frequency = <200000000>;
+
+		flash at 0 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			compatible = "macronix,mx25l1606e", "jedec-flash";
+
+			spi-max-frequency = <50000000>;
+			reg = <0>;
+
+			partition at 0 {
+				label = "U-Boot";
+				reg = <0x0 0x100000>;
+			};
+
+			partition at 100000 {
+				label = "U-Boot Environment";
+				reg = <0x100000 0x080000>;
+			};
+
+			partition at 180000 {
+				label = "Flattened Device Tree";
+				reg = <0x180000 0x080000>;
+			};
+		};
+	};
 };

(quoted diff) +0 -0

b/arch/arm/mach-kirkwood/board-dt.c index fbe6405..4960e63 100644

arch/arm/mach-kirkwood/board-dt.c +0 -40

@@ -39,42 +39,6 @@ static struct of_device_id kirkwood_dt_match_table[]
 __initdata = { { }
 };
 
-struct mtd_partition dreamplug_partitions[] = {
-	{
-		.name	= "u-boot",
-		.size	= SZ_512K,
-		.offset = 0,
-	},
-	{
-		.name	= "u-boot env",
-		.size	= SZ_64K,
-		.offset = SZ_512K + SZ_512K,
-	},
-	{
-		.name	= "dtb",
-		.size	= SZ_64K,
-		.offset = SZ_512K + SZ_512K + SZ_512K,
-	},
-};
-
-static const struct flash_platform_data dreamplug_spi_slave_data = {
-	.type		= "mx25l1606e",
-	.name		= "spi_flash",
-	.parts		= dreamplug_partitions,
-	.nr_parts	= ARRAY_SIZE(dreamplug_partitions),
-};
-
-static struct spi_board_info __initdata dreamplug_spi_slave_info[] = {
-	{
-		.modalias	= "m25p80",
-		.platform_data	= &dreamplug_spi_slave_data,
-		.irq		= -1,
-		.max_speed_hz	= 50000000,
-		.bus_num	= 0,
-		.chip_select	= 0,
-	},
-};
-
 static struct mv643xx_eth_platform_data dreamplug_ge00_data = {
 	.phy_addr	= MV643XX_ETH_PHY_ADDR(0),
 };

@@ -140,10 +104,6 @@ static void __init dreamplug_init(void)
 	 */
 	kirkwood_mpp_conf(dreamplug_mpp_config);
 
-	spi_register_board_info(dreamplug_spi_slave_info,
-				ARRAY_SIZE(dreamplug_spi_slave_info));
-	kirkwood_spi_init();
-
 	kirkwood_ehci_init();
 	kirkwood_ge00_init(&dreamplug_ge00_data);
 	kirkwood_ge01_init(&dreamplug_ge01_data);

(quoted diff) +0 -0

b/arch/arm/mach-kirkwood/common.c index cc15426..357fcde 100644

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

@@ -15,6 +15,7 @@
 #include <linux/ata_platform.h>
 #include <linux/mtd/nand.h>
 #include <linux/dma-mapping.h>
+#include <linux/of.h>
 #include <net/dsa.h>
 #include <asm/page.h>
 #include <asm/timex.h>

@@ -481,6 +482,9 @@ static int __init kirkwood_clock_gate(void)
 {
 	unsigned int curr = readl(CLOCK_GATING_CTRL);
 	u32 dev, rev;
+#ifdef CONFIG_OF
+	struct device_node *dp;
+#endif
 
 	kirkwood_pcie_id(&dev, &rev);
 	printk(KERN_DEBUG "Gating clock of unused units\n");

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

+		kirkwood_clk_ctrl |= CGC_RUNIT;
+#endif
+
 	/* Now gate clock the required units */
 	writel(kirkwood_clk_ctrl, CLOCK_GATING_CTRL);
 	printk(KERN_DEBUG " after: 0x%08x\n", readl(CLOCK_GATING_CTRL));

drivers/spi/spi-orion.c +4 -1

index 13448c8..acfb240 100644
@@ -18,6 +18,7 @@
 #include <linux/spi/spi.h>
 #include <linux/spi/orion_spi.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <asm/unaligned.h>
 
 #define DRIVER_NAME			"orion_spi"

@@ -45,6 +46,8 @@ struct orion_spi {
 	void __iomem		*base;
 	unsigned int		max_speed;
 	unsigned int		min_speed;
+	unsigned int		tclk;
+	unsigned int		clock_fix;
 	struct orion_spi_info	*spi_info;
 };
 

@@ -104,7 +107,7 @@ static int orion_spi_baudrate_set(struct spi_device
 *spi, unsigned int speed)
 
 	orion_spi = spi_master_get_devdata(spi->master);
 
-	tclk_hz = orion_spi->spi_info->tclk;
+	tclk_hz = orion_spi->tclk;
 
 	/*

drivers/spi/spi-orion.c +2 -1

 	 * the supported rates are: 4,6,8...30

@@ -360,7 +363,7 @@ static int orion_spi_setup(struct spi_device *spi)
 	orion_spi = spi_master_get_devdata(spi->master);
 
 	/* Fix ac timing if required.   */
-	if (orion_spi->spi_info->enable_clock_fix)
+	if (orion_spi->clock_fix)
 		orion_spi_setbits(orion_spi, ORION_SPI_IF_CONFIG_REG,
 				  (1 << 14));
 

@@ -474,6 +477,7 @@ static int __init orion_spi_probe(struct
 platform_device *pdev) master->setup = orion_spi_setup;
 	master->transfer = orion_spi_transfer;
 	master->num_chipselect = ORION_NUM_CHIPSELECTS;
+	master->dev.of_node = pdev->dev.of_node;
 
 	dev_set_drvdata(&pdev->dev, master);
 

@@ -481,8 +485,26 @@ static int __init orion_spi_probe(struct
 platform_device *pdev) spi->master = master;
 	spi->spi_info = spi_info;
 

drivers/spi/spi-orion.c +12 -0

+		status = -EINVAL;
+		goto out;
+	}
+
+	spi->max_speed = DIV_ROUND_UP(spi->tclk, 4);
+	spi->min_speed = DIV_ROUND_UP(spi->tclk, 30);
+
+	if (spi_info)
+		spi->clock_fix = spi_info->enable_clock_fix;
+
+	if (of_find_property(master->dev.of_node, "spi-clock-fix", NULL))
+		spi->clock_fix = 1;
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (r == NULL) {

drivers/spi/spi-orion.c +1 -0

 static struct platform_driver orion_spi_driver = {
 	.driver = {
 		.name	= DRIVER_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(spi_orion_of_match_table),
 	},
 	.remove		= __exit_p(orion_spi_remove),
 };

Quoted 4 lines

drivers/spi/spi-orion.c +0 -0

@@ -481,8 +485,26 @@ static int __init orion_spi_probe(struct
 platform_device *pdev) spi->master = master;
      spi->spi_info = spi_info;
 

drivers/spi/spi-orion.c static int __init orion_spi_probe(struct platform_device *pdev) · line 495

Quoted 10 lines

drivers/spi/spi-orion.c +8 -2

-     spi->max_speed = DIV_ROUND_UP(spi_info->tclk, 4);
-     spi->min_speed = DIV_ROUND_UP(spi_info->tclk, 30);
+     if (spi_info)
+             spi->tclk = spi_info->tclk;
+
+     of_property_read_u32(master->dev.of_node,
+                             "clock-frequency", &spi->tclk);
+
+     if (!spi->tclk) {
+             dev_err(&pdev->dev, "cannot set clock rate\n");

shouldn't you check the return value of of_property_read_u32 instead? and report a more meaningful error message, eg no valid clock-frequency property in the OF case?

The comment at mach-kirkwood/common.c:469, says this device is in every kirkwood board. So, it is placed in kirkwood.dtsi.

Signed-off-by Jason Cooper <[email protected]>

---

Changes since v1:

- removed unnecessary of_*() code from _probe()
- use of_match_ptr()

 arch/arm/boot/dts/kirkwood.dtsi   |    6 ++++++
 arch/arm/mach-kirkwood/board-dt.c |    1 -
 arch/arm/mach-kirkwood/common.c   |    2 +-
 arch/arm/mach-kirkwood/common.h   |    1 -
 drivers/rtc/rtc-mv.c              |    7 +++++++
 5 files changed, 14 insertions(+), 3 deletions(-)

(quoted diff) +0 -0

b/arch/arm/boot/dts/kirkwood.dtsi index 771c6bb..5fb185c 100644

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

@@ -2,5 +2,11 @@
 
 / {
 	compatible = "marvell,kirkwood";
+
+	rtc at f1010300 {
+		compatible = "marvell,rtc";
+		reg = <0xf1010300 0x1f>;
+		interrupts = <53>;
+	};
 };
 

(quoted diff) +0 -0

b/arch/arm/mach-kirkwood/board-dt.c index 2a45656..6e18338 100644

arch/arm/mach-kirkwood/board-dt.c +0 -1

@@ -134,7 +134,6 @@ static void __init kirkwood_dt_init(void)
 #endif
 
 	/* internal devices that every board has */
-	kirkwood_rtc_init();
 	kirkwood_wdt_init();
 	kirkwood_xor0_init();
 	kirkwood_xor1_init();

(quoted diff) +0 -0

b/arch/arm/mach-kirkwood/common.c index 167b6c8..0c0375f 100644

arch/arm/mach-kirkwood/common.c +1 -0

@@ -163,7 +163,7 @@ void __init kirkwood_nand_init_rnb(struct mtd_partition
 *parts, int nr_parts,
 /*************************************************************************
 **** * SoC RTC
 
 **************************************************************************
 **/ -void __init kirkwood_rtc_init(void)
+static void __init kirkwood_rtc_init(void)
 {
 	orion_rtc_init(RTC_PHYS_BASE, IRQ_KIRKWOOD_RTC);
 }

(quoted diff) +0 -0

b/arch/arm/mach-kirkwood/common.h index c382447..ca08826 100644

arch/arm/mach-kirkwood/common.h +0 -1

@@ -53,7 +53,6 @@ void kirkwood_restart(char, const char *);
 
 char *kirkwood_id(void);
 void kirkwood_l2_init(void);
-void kirkwood_rtc_init(void);
 void kirkwood_wdt_init(void);
 void kirkwood_xor0_init(void);
 void kirkwood_xor1_init(void);

drivers/rtc/rtc-mv.c +1 -0

index 768e2ed..3108b18 100644
@@ -12,6 +12,7 @@
 #include <linux/bcd.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
 #include <linux/delay.h>
 #include <linux/gfp.h>
 #include <linux/module.h>

drivers/rtc/rtc-mv.c +1 -0

 static struct platform_driver mv_rtc_driver = {
 	.remove		= __exit_p(mv_rtc_remove),
 	.driver		= {
 		.name	= "rtc-mv",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(rtc_mv_of_match_table),
 	},
 };

drivers/rtc/rtc-mv.c static int __exit mv_rtc_remove(struct · line 302

Quoted 9 lines

drivers/rtc/rtc-mv.c +5 -0

@@ -294,11 +295,17 @@ static int __exit mv_rtc_remove(struct
 platform_device *pdev) return 0;
 }
 
+static struct of_device_id rtc_mv_of_match_table[] = {
+     { .compatible = "marvell,rtc", },
+     {}
+};
+

Isn't here an ifdef CONFIG_OF missing to prevent compiler warnings?

Quoted 11 lines · Michael Walle

drivers/rtc/rtc-mv.c static int __exit mv_rtc_remove(struct · line 302

Quoted 9 lines

drivers/rtc/rtc-mv.c +5 -0

@@ -294,11 +295,17 @@ static int __exit mv_rtc_remove(struct
 platform_device *pdev) return 0;
 }
 
+static struct of_device_id rtc_mv_of_match_table[] = {
+     { .compatible = "marvell,rtc", },
+     {}
+};
+

Isn't here an ifdef CONFIG_OF missing to prevent compiler warnings?

Yes, I guess so. I keep thinking that an __unused attribute would actually be more helpful here because it lets the compiler automatically discard the symbol when it's not used, but we still get potential compiler errors and warnings if something inside of the unused data is broken.

Review

Quoted 10 lines · Jason Cooper

(quoted diff) +10 -0

+       wdt_reg = (void __iomem *)TIMER_VIRT_BASE;
+
+       ret = of_address_to_resource(np, 0, &res);
+       if (ret) {
+               printk(KERN_ERR "invalid address\n");
+               return ret;
+       }
+
+       wdt_reg = ioremap(res.start, res.end - res.start);
+

This will break non-dt uses because you overwrite the wdt_reg value.

As I commented before, I think the best approach would be to provide a hardcoded resource for the device in the legacy case and just use platform_get_resource() to get the resource and ioremap it in either case.

of_address_to_resource() works only for the DT case, while platform_get_resource() works in either case.

You need to add something like

8<---

arch/arm/plat-orion/common.c +5 -2

index 089899a..d16f43f 100644
@@ -568,13 +568,17 @@ void __init orion_spi_1_init(unsigned long mapbase,
  ****************************************************************************/
 static struct orion_wdt_platform_data orion_wdt_data;
 
+static struct resource orion_wdt_resource =
+               DEFINE_RES_MEM(TIMER_PHYS_BASE, 0x28);
+
 static struct platform_device orion_wdt_device = {
        .name           = "orion_wdt",
        .id             = -1,
        .dev            = {
                .platform_data  = &orion_wdt_data,
        },
-       .num_resources  = 0,
+       .resources      = &orion_wdt_resource,
+       .num_resources  = 1,
 };
 
 void __init orion_wdt_init(unsigned long tclk)
---->8

and an appropriate definition for TIMER_PHYS_BASE.

Quoted 4 lines · Arnd Bergmann

As I commented before, I think the best approach would be to provide a hardcoded resource for the device in the legacy case and just use platform_get_resource() to get the resource and ioremap it in either case.

Sorry, my denseness got in the way. When you said hardcoded, I thought you meant #define TIMER_VIRT_BASE .

The following changes since commit 759a45185ac0e4dfaf8bbfcb390ec73aca4b7a34:

ARM: kirkwood: convert uart0 to devicetree. (2012-02-27 16:21:44 +0000)

are available in the git repository at:
git://git.infradead.org/users/jcooper/linux-kirkwood.git kirkwood_dt_test

Quoted 7 lines · Jason

The following changes since commit 759a45185ac0e4dfaf8bbfcb390ec73aca4b7a34:

ARM: kirkwood: convert uart0 to devicetree. (2012-02-27 16:21:44 +0000)

are available in the git repository at:
git://git.infradead.org/users/jcooper/linux-kirkwood.git kirkwood_dt_test

Applied to next/dt branch.

Quoted 9 lines · Arnd Bergmann

Quoted 7 lines · Jason

The following changes since commit 759a45185ac0e4dfaf8bbfcb390ec73aca4b7a34:

ARM: kirkwood: convert uart0 to devicetree. (2012-02-27 16:21:44 +0000)

are available in the git repository at:
git://git.infradead.org/users/jcooper/linux-kirkwood.git kirkwood_dt_test

Quoted 1 line

Applied to next/dt branch.

On second thought, I should probably wait for the maintainer to comment as well ;-)

Nico, does this look ok to you?

Quoted 12 lines · Arnd Bergmann

Quoted 10 lines · Arnd Bergmann

Quoted 7 lines · Jason

The following changes since commit 759a45185ac0e4dfaf8bbfcb390ec73aca4b7a34:

ARM: kirkwood: convert uart0 to devicetree. (2012-02-27 16:21:44 +0000)

are available in the git repository at:
git://git.infradead.org/users/jcooper/linux-kirkwood.git kirkwood_dt_test

Applied to next/dt branch.

Quoted 4 lines

On second thought, I should probably wait for the maintainer to comment as well ;-)

Nico, does this look ok to you?

The clock frequency part being hardcoded to 200000 in the common .dtsi file looks wrong. The clock may differ, and it used to (and should) be probed at run time, please see kirkwood_find_tclk().

Quoted 4 lines

The clock frequency part being hardcoded to 200000 in the common .dtsi file looks wrong. The clock may differ, and it used to (and should) be probed at run time, please see kirkwood_find_tclk().

So, should I EXPORT_SYMBOL_GPL(kirkwood_find_tclk); and have each driver call it?

One alternate idea I had was to have kirkwood-dreamplug.dts root hold

tclk: clock-frequency = <200000000>;

and then have the drivers ask for it. afaict, tclk is constant per board.

I'm fine with either way, but I'd rather have it in the dt.

Quoted 7 lines · Jason

Quoted 4 lines

The clock frequency part being hardcoded to 200000 in the common .dtsi file looks wrong. The clock may differ, and it used to (and should) be probed at run time, please see kirkwood_find_tclk().

So, should I EXPORT_SYMBOL_GPL(kirkwood_find_tclk); and have each driver call it?

If the drivers want to use it, I think it has to be orion_find_tclk for drivers that are shared between multiple plat-orion platforms.

Quoted 4 lines

One alternate idea I had was to have kirkwood-dreamplug.dts root hold

tclk: clock-frequency = <200000000>;

Quoted 4 lines

and then have the drivers ask for it. afaict, tclk is constant per board.

I'm fine with either way, but I'd rather have it in the dt.

IMHO it's best to leave any such hacks out of the device tree for now, until we have the proper clk bindings in place for kirkwood, and then we can just use those.

Quoted 9 lines · Arnd Bergmann

Quoted 7 lines · Jason

Quoted 4 lines

The clock frequency part being hardcoded to 200000 in the common .dtsi file looks wrong. The clock may differ, and it used to (and should) be probed at run time, please see kirkwood_find_tclk().

So, should I EXPORT_SYMBOL_GPL(kirkwood_find_tclk); and have each driver call it?

Quoted 2 lines

If the drivers want to use it, I think it has to be orion_find_tclk for drivers that are shared between multiple plat-orion platforms.

That's pretty much all of them :-)

I'll rename it and move it to plat-orion/common.c. This'll be fun.

Quoted 13 lines · Jason

Quoted 11 lines · Arnd Bergmann

Quoted 7 lines · Jason

Quoted 4 lines

The clock frequency part being hardcoded to 200000 in the common .dtsi file looks wrong. The clock may differ, and it used to (and should) be probed at run time, please see kirkwood_find_tclk().

So, should I EXPORT_SYMBOL_GPL(kirkwood_find_tclk); and have each driver call it?

If the drivers want to use it, I think it has to be orion_find_tclk for drivers that are shared between multiple plat-orion platforms.

Quoted 3 lines

That's pretty much all of them :-)

I'll rename it and move it to plat-orion/common.c. This'll be fun.

No no... This is not a function which is generic at all. The _result_ i.e. core clock frequency value is a generic thing, not the method to determine it.

Quoted 19 lines · Nicolas Pitre

Quoted 16 lines · Jason

Quoted 11 lines · Arnd Bergmann

Quoted 7 lines · Jason

Quoted 4 lines

The clock frequency part being hardcoded to 200000 in the common .dtsi file looks wrong. The clock may differ, and it used to (and should) be probed at run time, please see kirkwood_find_tclk().

So, should I EXPORT_SYMBOL_GPL(kirkwood_find_tclk); and have each driver call it?

If the drivers want to use it, I think it has to be orion_find_tclk for drivers that are shared between multiple plat-orion platforms.

That's pretty much all of them :-)

I'll rename it and move it to plat-orion/common.c. This'll be fun.

Quoted 3 lines

No no... This is not a function which is generic at all. The _result_ i.e. core clock frequency value is a generic thing, not the method to determine it.

How about this? Export a global variable, orion_tclk, and a function to read it. Then, make sure each sub arch sets the global. Does this sound viable until common clk lands?

Quoted 24 lines · Jason

Quoted 22 lines · Nicolas Pitre

Quoted 10 lines · Jason

Quoted 9 lines · Arnd Bergmann

Quoted 7 lines · Jason

Quoted 4 lines

The clock frequency part being hardcoded to 200000 in the common .dtsi file looks wrong. The clock may differ, and it used to (and should) be probed at run time, please see kirkwood_find_tclk().

So, should I EXPORT_SYMBOL_GPL(kirkwood_find_tclk); and have each driver call it?

Quoted 6 lines

Quoted 2 lines

If the drivers want to use it, I think it has to be orion_find_tclk for drivers that are shared between multiple plat-orion platforms.

That's pretty much all of them :-)

I'll rename it and move it to plat-orion/common.c. This'll be fun.

No no... This is not a function which is generic at all. The _result_ i.e. core clock frequency value is a generic thing, not the method to determine it.

Quoted 3 lines

How about this? Export a global variable, orion_tclk, and a function to read it. Then, make sure each sub arch sets the global. Does this sound viable until common clk lands?

Yep, looks fine.

Review

Quoted 10 lines · Jason Cooper

(quoted diff) +10 -0

+       wdt_reg = (void __iomem *)TIMER_VIRT_BASE;
+
+       ret = of_address_to_resource(np, 0, &res);
+       if (ret) {
+               printk(KERN_ERR "invalid address\n");
+               return ret;
+       }
+
+       wdt_reg = ioremap(res.start, res.end - res.start);
+

This will break non-dt uses because you overwrite the wdt_reg value.

Quoted 13 lines · Arnd Bergmann

Review

Quoted 10 lines · Jason Cooper

(quoted diff) +10 -0

+       wdt_reg = (void __iomem *)TIMER_VIRT_BASE;
+
+       ret = of_address_to_resource(np, 0, &res);
+       if (ret) {
+               printk(KERN_ERR "invalid address\n");
+               return ret;
+       }
+
+       wdt_reg = ioremap(res.start, res.end - res.start);
+

This will break non-dt uses because you overwrite the wdt_reg value.

Don't also forget that "res.end - res.start" is also not correct, and resource_size() should be used to prevent that kind of error.

The following changes since commit 759a45185ac0e4dfaf8bbfcb390ec73aca4b7a34:

ARM: kirkwood: convert uart0 to devicetree. (2012-02-27 16:21:44 +0000)

are available in the git repository at:
git://git.infradead.org/users/jcooper/linux-kirkwood.git kirkwood_dt_test

Jason Cooper (5):
ARM: kirkwood: covert orion-spi to fdt.
ARM: kirkwood: move var setting to correct location.
ARM: kirkwood: fdt: absorb kirkwood_init()
ARM: kirkwood: convert rtc-mv to fdt.
ARM: kirkwood: convert orion-wdt to fdt.

 arch/arm/boot/dts/kirkwood-dreamplug.dts |   34 +++++++++++++++
 arch/arm/boot/dts/kirkwood.dtsi          |   12 +++++
 arch/arm/mach-kirkwood/board-dt.c        |   67 +++++++++++------------------
 arch/arm/mach-kirkwood/common.c          |   21 +++++++--
 arch/arm/mach-kirkwood/common.h          |    6 +++
 drivers/rtc/rtc-mv.c                     |    7 +++
 drivers/spi/spi-orion.c                  |   37 +++++++++++++++--
 drivers/watchdog/orion_wdt.c             |   48 ++++++++++++++++-----
 8 files changed, 170 insertions(+), 62 deletions(-)

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

From: Andrew Lunn <[email protected]>

Signed-off-by Andrew Lunn <[email protected]>

Signed-off-by Jason Cooper <[email protected]>

---
 drivers/spi/spi-orion.c       |    5 -----
 include/linux/spi/orion_spi.h |    1 -
 2 files changed, 0 insertions(+), 6 deletions(-)

drivers/spi/spi-orion.c +0 -5

index 13448c8..e496f79 100644
@@ -359,11 +359,6 @@ static int orion_spi_setup(struct spi_device *spi)
 
 	orion_spi = spi_master_get_devdata(spi->master);
 
-	/* Fix ac timing if required.   */
-	if (orion_spi->spi_info->enable_clock_fix)
-		orion_spi_setbits(orion_spi, ORION_SPI_IF_CONFIG_REG,
-				  (1 << 14));
-
 	if ((spi->max_speed_hz == 0)
 			|| (spi->max_speed_hz > orion_spi->max_speed))
 		spi->max_speed_hz = orion_spi->max_speed;

include/linux/spi/orion_spi.h +0 -1

index decf6d8..b4d9fa6 100644
@@ -11,7 +11,6 @@
 
 struct orion_spi_info {
 	u32	tclk;		/* no <linux/clk.h> support yet */
-	u32	enable_clock_fix;
 };
 
 

Signature · 1 line

1.7.3.4