[PATCHv2 1/1] ARM:vt8500: Convert to use .restart and remove arch_reset()

Changed the existing board files to use .restart in there machine
descriptions.
Removed system.h as it only contained an inline for arch_reset()

Added device tree support for the restart controller.
Device tree support for vt8500 is still a work-in-progress.

Signed-off-by: Tony Prisk <redacted>
---
?arch/arm/mach-vt8500/Makefile?????????????? |??? 2 +-
?arch/arm/mach-vt8500/bv07.c???????????????? |??? 3 +
?arch/arm/mach-vt8500/include/mach/restart.h |?? 17 +++++++
?arch/arm/mach-vt8500/include/mach/system.h? |?? 13 -----
?arch/arm/mach-vt8500/restart.c????????????? |?? 64 +++++++++++++++++++++++++++
?arch/arm/mach-vt8500/wm8505_7in.c?????????? |??? 4 +-
?6 files changed, 88 insertions(+), 15 deletions(-)
?create mode 100644 arch/arm/mach-vt8500/include/mach/restart.h
?delete mode 100644 arch/arm/mach-vt8500/include/mach/system.h
?create mode 100644 arch/arm/mach-vt8500/restart.c

diff --git a/arch/arm/mach-vt8500/Makefile b/arch/arm/mach-vt8500/Makefile
index 81aedb7..54e6997 100644
--- a/arch/arm/mach-vt8500/Makefile
+++ b/arch/arm/mach-vt8500/Makefile

@@ -1,4 +1,4 @@
-obj-y += devices.o gpio.o irq.o timer.o
+obj-y += devices.o gpio.o irq.o timer.o restart.o

?
?obj-$(CONFIG_VTWM_VERSION_VT8500) += devices-vt8500.o
?obj-$(CONFIG_VTWM_VERSION_WM8505) += devices-wm8505.o

diff --git a/arch/arm/mach-vt8500/bv07.c b/arch/arm/mach-vt8500/bv07.c
index a464c75..f9fbeb2 100644
--- a/arch/arm/mach-vt8500/bv07.c
+++ b/arch/arm/mach-vt8500/bv07.c

@@ -23,6 +23,7 @@

?
?#include <asm/mach-types.h>
?#include <asm/mach/arch.h>
+#include <mach/restart.h>
?
?#include "devices.h"
?

@@ -62,6 +63,7 @@ void __init bv07_init(void)

???? else
???? ??? printk(KERN_ERR "PMC Hibernation register could not be remapped, not enabling power off!\n");
?
+??? wmt_setup_restart();
???? vt8500_set_resources();
???? platform_add_devices(devices, ARRAY_SIZE(devices));
???? vt8500_gpio_init();

@@ -69,6 +71,7 @@ void __init bv07_init(void)

?
?MACHINE_START(BV07, "Benign BV07 Mini Netbook")
???? .atag_offset??? = 0x100,
+??? .restart??? = wmt_restart,
???? .reserve??? = vt8500_reserve_mem,
???? .map_io??? ??? = vt8500_map_io,
???? .init_irq??? = vt8500_init_irq,

diff --git a/arch/arm/mach-vt8500/include/mach/restart.h b/arch/arm/mach-vt8500/include/mach/restart.h
new file mode 100644
index 0000000..89f9b78
--- /dev/null
+++ b/arch/arm/mach-vt8500/include/mach/restart.h

@@ -0,0 +1,17 @@
+/* linux/arch/arm/mach-vt8500/restart.h
+ *
+ * Copyright (C) 2012 Tony Prisk <linux@prisktech.co.nz>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.? See the
+ * GNU General Public License for more details.
+ *
+ */
+
+void wmt_setup_restart(void);
+void wmt_restart(char mode, const char *cmd);

diff --git a/arch/arm/mach-vt8500/include/mach/system.h b/arch/arm/mach-vt8500/include/mach/system.h
deleted file mode 100644
index 58fa801..0000000
--- a/arch/arm/mach-vt8500/include/mach/system.h
+++ /dev/null

@@ -1,13 +0,0 @@
-/*
- * arch/arm/mach-vt8500/include/mach/system.h
- *
- */
-#include <asm/io.h>
-
-/* PM Software Reset request register */
-#define VT8500_PMSR_VIRT??? 0xf8130060
-
-static inline void arch_reset(char mode, const char *cmd)
-{
-??? writel(1, VT8500_PMSR_VIRT);
-}

diff --git a/arch/arm/mach-vt8500/restart.c b/arch/arm/mach-vt8500/restart.c
new file mode 100644
index 0000000..9134aea
--- /dev/null
+++ b/arch/arm/mach-vt8500/restart.c

@@ -0,0 +1,64 @@
+/* linux/arch/arm/mach-vt8500/restart.c
+ *
+ * Copyright (C) 2012 Tony Prisk <linux@prisktech.co.nz>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.? See the
+ * GNU General Public License for more details.
+ *
+ */
+#include <asm/io.h>
+#include <linux/ioport.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+/* PM Software Reset request register */
+#define LEGACY_PMSR_REG??? ??? 0xD8130060
+#define WMT_PRIZM_PMSR_REG??? 0x60
+
+static void __iomem *wmt_restart_reg;
+
+void wmt_setup_restart(void)
+{
+??? struct device_node *np;
+??? void __iomem *pmc_base;
+
+??? /*
+??? ?* Check if Power Mgmt Controller node is present in device tree. If no
+??? ?* device tree node, use the legacy PMSR value (valid for all current
+??? ?* SoCs).
+??? ?*/
+??? np = of_find_compatible_node(NULL, NULL, "wmt,prizm-pmc");
+??? if (np) {
+??? ??? pmc_base = of_iomap(np, 0);
+
+??? ??? if (!pmc_base)
+??? ??? ??? pr_err("%s:of_iomap(pmc) failed\n", __func__);
+
+??? ??? of_node_put(np);
+
+??? ??? wmt_restart_reg = (void *)((u32)(pmc_base) + WMT_PRIZM_PMSR_REG);
+??? } else {
+??? ??? if (!request_mem_region(LEGACY_PMSR_REG, 4, NULL)) {
+??? ??? ??? pr_err("%s:request_mem_region(rst) failed\n", __func__);
+??? ??? ??? return;
+??? ??? }
+
+??? ??? wmt_restart_reg = ioremap(LEGACY_PMSR_REG, 4);
+??? ??? if (!wmt_restart_reg) {
+??? ??? ??? pr_err("%s:ioremap(rstc) failed\n", __func__);
+??? ??? ??? return;
+??? ??? }
+??? }
+}
+
+void wmt_restart(char mode, const char *cmd)
+{
+??? if (wmt_restart_reg)
+??? ??? writel(1, wmt_restart_reg);
+}

diff --git a/arch/arm/mach-vt8500/wm8505_7in.c b/arch/arm/mach-vt8500/wm8505_7in.c
index cf910a9..db19886 100644
--- a/arch/arm/mach-vt8500/wm8505_7in.c
+++ b/arch/arm/mach-vt8500/wm8505_7in.c

@@ -23,6 +23,7 @@

?
?#include <asm/mach-types.h>
?#include <asm/mach/arch.h>
+#include <mach/restart.h>
?
?#include "devices.h"
?

@@ -61,7 +62,7 @@ void __init wm8505_7in_init(void)

???? ??? pm_power_off = &vt8500_power_off;
???? else
???? ??? printk(KERN_ERR "PMC Hibernation register could not be remapped, not enabling power off!\n");
-
+??? wmt_setup_restart();
???? wm8505_set_resources();
???? platform_add_devices(devices, ARRAY_SIZE(devices));
???? vt8500_gpio_init();

@@ -69,6 +70,7 @@ void __init wm8505_7in_init(void)

?
?MACHINE_START(WM8505_7IN_NETBOOK, "WM8505 7-inch generic netbook")
???? .atag_offset??? = 0x100,
+??? .restart??? = wmt_restart,
???? .reserve??? = wm8505_reserve_mem,
???? .map_io??? ??? = wm8505_map_io,
???? .init_irq??? = wm8505_init_irq,
-- 
1.7.2.5

2012/7/18 Tony Prisk [off-list ref]:

Changed the existing board files to use .restart in there machine

Looks like a typo here :)

GCC allows pointer arithmetics on void * directly, and that's relied
upon heavily throughout the kernel code, so you don't need to cast it
here.

+    } else {
+        if (!request_mem_region(LEGACY_PMSR_REG, 4, NULL)) {

Is this required? I believe we could drop it just as well.

Thanks,
Alexey

On Wednesday 18 July 2012, Tony Prisk wrote:

Changed the existing board files to use .restart in there machine
descriptions.
Removed system.h as it only contained an inline for arch_reset()

Added device tree support for the restart controller.
Device tree support for vt8500 is still a work-in-progress.

Signed-off-by: Tony Prisk <redacted>

Looks ok from the point of view that it's technically correct.
Let me make some comments about the style though:

+/* PM Software Reset request register */
+#define LEGACY_PMSR_REG        0xD8130060
+#define WMT_PRIZM_PMSR_REG    0x60
+
+static void __iomem *wmt_restart_reg;

I think it's better to point this to the pmc_base virtual address,
which is what we commonly do for similar scenarios. This also means
changing the phys address constant here to 0xD8130000, i.e. the
same as the one in the device tree.

+    /*
+     * Check if Power Mgmt Controller node is present in device tree. If no
+     * device tree node, use the legacy PMSR value (valid for all current
+     * SoCs).
+     */
+    np = of_find_compatible_node(NULL, NULL, "wmt,prizm-pmc");
+    if (np) {
+        pmc_base = of_iomap(np, 0);
+
+        if (!pmc_base)
+            pr_err("%s:of_iomap(pmc) failed\n", __func__);
+
+        of_node_put(np);
+
+        wmt_restart_reg = (void *)((u32)(pmc_base) + WMT_PRIZM_PMSR_REG);

The type cast is ugly, wrong and uncessary ;-)

It's ugly because you are casting the same address twice, which you should
never need to do.
It's wrong because you use the wrong target type (should be void __iomem *)
and the intermediate type (u32) relies on the code being compiled for 32
bit. This is probably a safe assumption, but if you have to cast between
a pointer and an integer in the kernel, the idiom is to use 'unsigned long'
as the integer type.

It's also unnecessary because 

	wmt_restart_reg = pmc_base + WMT_PRIZM_PMSR_REG;

has the exact same effect without any of the above disadvantages.

As mentioned, I would not do the calculation here at all but instead
do it below in wmt_restart(), as 

	if (pmc_base)
		writel(1, pmc_base + WMT_PRIZM_PMSR_REG);

	Arnd

Removed system.h as it only contained an inline for arch_reset()
Changed the existing board files to use .restart in machine descriptions.

Added device tree support for the restart controller.
Device tree support for mach-vt8500 is still a work-in-progress.

Change History
v3: style changes.

Signed-off-by: Tony Prisk <redacted>
---
?arch/arm/mach-vt8500/Makefile?????????????? |??? 2 +-
?arch/arm/mach-vt8500/bv07.c???????????????? |??? 3 +
?arch/arm/mach-vt8500/include/mach/restart.h |?? 17 ++++++++
?arch/arm/mach-vt8500/include/mach/system.h? |?? 13 ------
?arch/arm/mach-vt8500/restart.c????????????? |?? 54 +++++++++++++++++++++++++++
?arch/arm/mach-vt8500/wm8505_7in.c?????????? |??? 4 +-
?6 files changed, 78 insertions(+), 15 deletions(-)
?create mode 100644 arch/arm/mach-vt8500/include/mach/restart.h
?delete mode 100644 arch/arm/mach-vt8500/include/mach/system.h
?create mode 100644 arch/arm/mach-vt8500/restart.c

diff --git a/arch/arm/mach-vt8500/Makefile b/arch/arm/mach-vt8500/Makefile
index 81aedb7..54e6997 100644
--- a/arch/arm/mach-vt8500/Makefile
+++ b/arch/arm/mach-vt8500/Makefile

@@ -1,4 +1,4 @@
-obj-y += devices.o gpio.o irq.o timer.o
+obj-y += devices.o gpio.o irq.o timer.o restart.o

?
?obj-$(CONFIG_VTWM_VERSION_VT8500) += devices-vt8500.o
?obj-$(CONFIG_VTWM_VERSION_WM8505) += devices-wm8505.o

diff --git a/arch/arm/mach-vt8500/bv07.c b/arch/arm/mach-vt8500/bv07.c
index a464c75..f9fbeb2 100644
--- a/arch/arm/mach-vt8500/bv07.c
+++ b/arch/arm/mach-vt8500/bv07.c

@@ -23,6 +23,7 @@

?
?#include <asm/mach-types.h>
?#include <asm/mach/arch.h>
+#include <mach/restart.h>
?
?#include "devices.h"
?

@@ -62,6 +63,7 @@ void __init bv07_init(void)

???? else
???? ??? printk(KERN_ERR "PMC Hibernation register could not be remapped, not enabling power off!\n");
?
+??? wmt_setup_restart();
???? vt8500_set_resources();
???? platform_add_devices(devices, ARRAY_SIZE(devices));
???? vt8500_gpio_init();

@@ -69,6 +71,7 @@ void __init bv07_init(void)

?
?MACHINE_START(BV07, "Benign BV07 Mini Netbook")
???? .atag_offset??? = 0x100,
+??? .restart??? = wmt_restart,
???? .reserve??? = vt8500_reserve_mem,
???? .map_io??? ??? = vt8500_map_io,
???? .init_irq??? = vt8500_init_irq,

diff --git a/arch/arm/mach-vt8500/include/mach/restart.h b/arch/arm/mach-vt8500/include/mach/restart.h
new file mode 100644
index 0000000..89f9b78
--- /dev/null
+++ b/arch/arm/mach-vt8500/include/mach/restart.h

@@ -0,0 +1,17 @@
+/* linux/arch/arm/mach-vt8500/restart.h
+ *
+ * Copyright (C) 2012 Tony Prisk <linux@prisktech.co.nz>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.? See the
+ * GNU General Public License for more details.
+ *
+ */
+
+void wmt_setup_restart(void);
+void wmt_restart(char mode, const char *cmd);

diff --git a/arch/arm/mach-vt8500/include/mach/system.h b/arch/arm/mach-vt8500/include/mach/system.h
deleted file mode 100644
index 58fa801..0000000
--- a/arch/arm/mach-vt8500/include/mach/system.h
+++ /dev/null

@@ -1,13 +0,0 @@
-/*
- * arch/arm/mach-vt8500/include/mach/system.h
- *
- */
-#include <asm/io.h>
-
-/* PM Software Reset request register */
-#define VT8500_PMSR_VIRT??? 0xf8130060
-
-static inline void arch_reset(char mode, const char *cmd)
-{
-??? writel(1, VT8500_PMSR_VIRT);
-}

diff --git a/arch/arm/mach-vt8500/restart.c b/arch/arm/mach-vt8500/restart.c
new file mode 100644
index 0000000..497e89a
--- /dev/null
+++ b/arch/arm/mach-vt8500/restart.c

@@ -0,0 +1,54 @@
+/* linux/arch/arm/mach-vt8500/restart.c
+ *
+ * Copyright (C) 2012 Tony Prisk <linux@prisktech.co.nz>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.? See the
+ * GNU General Public License for more details.
+ *
+ */
+#include <asm/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#define LEGACY_PMC_BASE??? ??? 0xD8130000
+#define WMT_PRIZM_PMSR_REG??? 0x60
+
+static void __iomem *pmc_base;
+
+void wmt_setup_restart(void)
+{
+??? struct device_node *np;
+
+??? /*
+??? ?* Check if Power Mgmt Controller node is present in device tree. If no
+??? ?* device tree node, use the legacy PMSR value (valid for all current
+??? ?* SoCs).
+??? ?*/
+??? np = of_find_compatible_node(NULL, NULL, "wmt,prizm-pmc");
+??? if (np) {
+??? ??? pmc_base = of_iomap(np, 0);
+
+??? ??? if (!pmc_base)
+??? ??? ??? pr_err("%s:of_iomap(pmc) failed\n", __func__);
+
+??? ??? of_node_put(np);
+??? } else {
+??? ??? pmc_base = ioremap(LEGACY_PMC_BASE, 0x1000);
+??? ??? if (!pmc_base) {
+??? ??? ??? pr_err("%s:ioremap(rstc) failed\n", __func__);
+??? ??? ??? return;
+??? ??? }
+??? }
+}
+
+void wmt_restart(char mode, const char *cmd)
+{
+??? if (pmc_base)
+??? ??? writel(1, pmc_base + WMT_PRIZM_PMSR_REG);
+}

diff --git a/arch/arm/mach-vt8500/wm8505_7in.c b/arch/arm/mach-vt8500/wm8505_7in.c
index cf910a9..db19886 100644
--- a/arch/arm/mach-vt8500/wm8505_7in.c
+++ b/arch/arm/mach-vt8500/wm8505_7in.c

@@ -23,6 +23,7 @@

?
?#include <asm/mach-types.h>
?#include <asm/mach/arch.h>
+#include <mach/restart.h>
?
?#include "devices.h"
?

@@ -61,7 +62,7 @@ void __init wm8505_7in_init(void)

???? ??? pm_power_off = &vt8500_power_off;
???? else
???? ??? printk(KERN_ERR "PMC Hibernation register could not be remapped, not enabling power off!\n");
-
+??? wmt_setup_restart();
???? wm8505_set_resources();
???? platform_add_devices(devices, ARRAY_SIZE(devices));
???? vt8500_gpio_init();

@@ -69,6 +70,7 @@ void __init wm8505_7in_init(void)

?
?MACHINE_START(WM8505_7IN_NETBOOK, "WM8505 7-inch generic netbook")
???? .atag_offset??? = 0x100,
+??? .restart??? = wmt_restart,
???? .reserve??? = wm8505_reserve_mem,
???? .map_io??? ??? = wm8505_map_io,
???? .init_irq??? = wm8505_init_irq,
-- 
1.7.2.5

On Wednesday 18 July 2012, Tony Prisk wrote:

Removed system.h as it only contained an inline for arch_reset()
Changed the existing board files to use .restart in machine descriptions.

Added device tree support for the restart controller.
Device tree support for mach-vt8500 is still a work-in-progress.

Change History
v3: style changes.

The change history of the patch should go below the '---'
line, there is no need to preserve that in the git changelog
in cases like this. For larger patches you may actually
want to preserve it and put it here, but the fact that you
applied style changes is not such a case ;-)

Signed-off-by: Tony Prisk <redacted>

Acked-by: Arnd Bergmann <arnd@arndb.de>

The patch looks good, but you've sent it through a mail client
that mangles the patch into a form that I cannot apply.

Please use git-send-email or use a setup described in 
Documentation/email-clients.txt.

The preferred way would actually be a 'git request-pull'
with a public git URL, but patches are also fine
if it's only an occasional bug fix like this.

	Arnd

On Wednesday 18 July 2012, Tony Prisk wrote:

Removed system.h as it only contained an inline for arch_reset()
Changed the existing board files to use .restart in machine descriptions.

Added device tree support for the restart controller.
Device tree support for mach-vt8500 is still a work-in-progress.

Change History
v3: style changes.

Signed-off-by: Tony Prisk <redacted>

Applied the new version now, thanks a lot!

	Arnd