[PATCHv2 0/5] Misc picoxcell updates for 3.3

[PATCHv2 0/5] Misc picoxcell updates for 3.3

[Jamie Iles]

Here's v2 of a small series of feature updates for picoxcell in 3.3.

Jamie

Jamie Iles (5):
  ARM: picoxcell: remove mach/memory.h
  ARM: picoxcell: don't reserve irq_descs
  ARM: picoxcell: move io mappings to common.c
  MAINTAINERS: add maintainer entry for Picochip picoxcell
  ARM: picoxcell: implement watchdog restart

 MAINTAINERS                                   |    9 +++++
 arch/arm/Kconfig                              |    1 +
 arch/arm/mach-picoxcell/Makefile              |    1 -
 arch/arm/mach-picoxcell/common.c              |   47 ++++++++++++++++++++++++-
 arch/arm/mach-picoxcell/common.h              |    1 -
 arch/arm/mach-picoxcell/include/mach/irqs.h   |    9 +----
 arch/arm/mach-picoxcell/include/mach/memory.h |    1 -
 arch/arm/mach-picoxcell/io.c                  |   32 -----------------
 8 files changed, 58 insertions(+), 43 deletions(-)
 delete mode 100644 arch/arm/mach-picoxcell/include/mach/memory.h
 delete mode 100644 arch/arm/mach-picoxcell/io.c

-- 
1.7.5.4

[PATCHv2 1/5] ARM: picoxcell: remove mach/memory.h

[Jamie Iles]

mach/memory.h is no longer required for simple platforms so remove it
for picoxcell.

Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 arch/arm/mach-picoxcell/include/mach/memory.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
 delete mode 100644 arch/arm/mach-picoxcell/include/mach/memory.h

diff --git a/arch/arm/mach-picoxcell/include/mach/memory.h b/arch/arm/mach-picoxcell/include/mach/memory.h
deleted file mode 100644
index 40a8c17..0000000
--- a/arch/arm/mach-picoxcell/include/mach/memory.h
+++ /dev/null
@@ -1 +0,0 @@
-/* empty */
-- 
1.7.5.4

[PATCHv2 2/5] ARM: picoxcell: don't reserve irq_descs

[Jamie Iles]

All irq_desc's are now dynamically allocated so we don't need to
statically reserve them.

v2:	- select SPARSE_IRQ and set .nr_irqs to NR_IRQS_LEGACY to skip
	ISA and IRQ 0.

Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 arch/arm/Kconfig                            |    1 +
 arch/arm/mach-picoxcell/common.c            |    2 +-
 arch/arm/mach-picoxcell/include/mach/irqs.h |    9 ++-------
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 44789ef..0831d48 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -650,6 +650,7 @@ config ARCH_PICOXCELL
 	select HAVE_SCHED_CLOCK
 	select HAVE_TCM
 	select NO_IOPORT
+	select SPARSE_IRQ
 	select USE_OF
 	help
 	  This enables support for systems based on the Picochip picoXcell
diff --git a/arch/arm/mach-picoxcell/common.c b/arch/arm/mach-picoxcell/common.c
index ad871bd..d34b333 100644
--- a/arch/arm/mach-picoxcell/common.c
+++ b/arch/arm/mach-picoxcell/common.c
@@ -45,7 +45,7 @@ static void __init picoxcell_init_irq(void)
 
 DT_MACHINE_START(PICOXCELL, "Picochip picoXcell")
 	.map_io		= picoxcell_map_io,
-	.nr_irqs	= ARCH_NR_IRQS,
+	.nr_irqs	= NR_IRQS_LEGACY,
 	.init_irq	= picoxcell_init_irq,
 	.handle_irq	= vic_handle_irq,
 	.timer		= &picoxcell_timer,
diff --git a/arch/arm/mach-picoxcell/include/mach/irqs.h b/arch/arm/mach-picoxcell/include/mach/irqs.h
index 4d13ed9..59eac1e 100644
--- a/arch/arm/mach-picoxcell/include/mach/irqs.h
+++ b/arch/arm/mach-picoxcell/include/mach/irqs.h
@@ -1,8 +1,6 @@
 /*
  * Copyright (c) 2011 Picochip Ltd., Jamie Iles
  *
- * This file contains the hardware definitions of the picoXcell SoC devices.
- *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
@@ -16,10 +14,7 @@
 #ifndef __MACH_IRQS_H
 #define __MACH_IRQS_H
 
-#define ARCH_NR_IRQS			64
-#define NR_IRQS				(128 + ARCH_NR_IRQS)
-
-#define IRQ_VIC0_BASE			0
-#define IRQ_VIC1_BASE			32
+/* We dynamically allocate our irq_desc's. */
+#define NR_IRQS				0
 
 #endif /* __MACH_IRQS_H */
-- 
1.7.5.4

[PATCHv2 3/5] ARM: picoxcell: move io mappings to common.c

[Jamie Iles]

Now that we have lost our machine specific ioremap() we just have one
mapping that covers all peripherals.  Move this to common.c to simplify
things a little.

Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 arch/arm/mach-picoxcell/Makefile |    1 -
 arch/arm/mach-picoxcell/common.c |   13 +++++++++++++
 arch/arm/mach-picoxcell/common.h |    1 -
 arch/arm/mach-picoxcell/io.c     |   32 --------------------------------
 4 files changed, 13 insertions(+), 34 deletions(-)
 delete mode 100644 arch/arm/mach-picoxcell/io.c

diff --git a/arch/arm/mach-picoxcell/Makefile b/arch/arm/mach-picoxcell/Makefile
index c550b63..e5ec4a8 100644
--- a/arch/arm/mach-picoxcell/Makefile
+++ b/arch/arm/mach-picoxcell/Makefile
@@ -1,3 +1,2 @@
 obj-y	:= common.o
 obj-y	+= time.o
-obj-y	+= io.o
diff --git a/arch/arm/mach-picoxcell/common.c b/arch/arm/mach-picoxcell/common.c
index d34b333..febee47 100644
--- a/arch/arm/mach-picoxcell/common.c
+++ b/arch/arm/mach-picoxcell/common.c
@@ -16,12 +16,25 @@
 
 #include <asm/mach/arch.h>
 #include <asm/hardware/vic.h>
+#include <asm/mach/map.h>
 
 #include <mach/map.h>
 #include <mach/picoxcell_soc.h>
 
 #include "common.h"
 
+static struct map_desc io_map __initdata = {
+	.virtual	= PHYS_TO_IO(PICOXCELL_PERIPH_BASE),
+	.pfn		= __phys_to_pfn(PICOXCELL_PERIPH_BASE),
+	.length		= PICOXCELL_PERIPH_LENGTH,
+	.type		= MT_DEVICE,
+};
+
+static void __init picoxcell_map_io(void)
+{
+	iotable_init(&io_map, 1);
+}
+
 static void __init picoxcell_init_machine(void)
 {
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
diff --git a/arch/arm/mach-picoxcell/common.h b/arch/arm/mach-picoxcell/common.h
index 5263f0f..83d55ab 100644
--- a/arch/arm/mach-picoxcell/common.h
+++ b/arch/arm/mach-picoxcell/common.h
@@ -13,6 +13,5 @@
 #include <asm/mach/time.h>
 
 extern struct sys_timer picoxcell_timer;
-extern void picoxcell_map_io(void);
 
 #endif /* __PICOXCELL_COMMON_H__ */
diff --git a/arch/arm/mach-picoxcell/io.c b/arch/arm/mach-picoxcell/io.c
deleted file mode 100644
index 39e9b9e..0000000
--- a/arch/arm/mach-picoxcell/io.c
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- * Copyright (c) 2011 Picochip Ltd., Jamie Iles
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * All enquiries to support at picochip.com
- */
-#include <linux/io.h>
-#include <linux/mm.h>
-#include <linux/module.h>
-#include <linux/of.h>
-
-#include <asm/mach/map.h>
-
-#include <mach/map.h>
-#include <mach/picoxcell_soc.h>
-
-#include "common.h"
-
-void __init picoxcell_map_io(void)
-{
-	struct map_desc io_map = {
-		.virtual	= PHYS_TO_IO(PICOXCELL_PERIPH_BASE),
-		.pfn		= __phys_to_pfn(PICOXCELL_PERIPH_BASE),
-		.length		= PICOXCELL_PERIPH_LENGTH,
-		.type		= MT_DEVICE,
-	};
-
-	iotable_init(&io_map, 1);
-}
-- 
1.7.5.4

[PATCHv2 4/5] MAINTAINERS: add maintainer entry for Picochip picoxcell

[Jamie Iles]

Add maintainer entry for the picoxcell machine support and associated
drivers.

v2:	- add 2-level pattern for drivers

Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 MAINTAINERS |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 071a996..85e5071 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5114,6 +5114,15 @@ L:	linux-mtd at lists.infradead.org
 S:	Maintained
 F:	drivers/mtd/devices/phram.c
 
+PICOXCELL SUPPORT
+M:	Jamie Iles <jamie@jamieiles.com>
+L:	linux-arm-kernel at lists.infradead.org (moderated for non-subscribers)
+T:	git git://github.com/jamieiles/linux-2.6-ji.git
+S:	Supported
+F:	arch/arm/mach-picoxcell
+F:	drivers/*/picoxcell*
+F:	drivers/*/*/picoxcell*
+
 PIN CONTROL SUBSYSTEM
 M:	Linus Walleij <linus.walleij@linaro.org>
 S:	Maintained
-- 
1.7.5.4

[PATCHv2 5/5] ARM: picoxcell: implement watchdog restart

[Jamie Iles]

Allow the platform to be restarted by triggering the watchdog to expire
with the shortest possible expiry.  This should reset the CPU core and
all on-chip peripherals.

Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 arch/arm/mach-picoxcell/common.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-picoxcell/common.c b/arch/arm/mach-picoxcell/common.c
index febee47..2e82de7 100644
--- a/arch/arm/mach-picoxcell/common.c
+++ b/arch/arm/mach-picoxcell/common.c
@@ -7,6 +7,7 @@
  *
  * All enquiries to support at picochip.com
  */
+#include <linux/delay.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/of.h>
@@ -23,6 +24,22 @@
 
 #include "common.h"
 
+#define WDT_CTRL_REG_EN_MASK	(1 << 0)
+#define WDT_CTRL_REG_OFFS	(0x00)
+#define WDT_TIMEOUT_REG_OFFS	(0x04)
+static void __iomem *wdt_regs;
+
+static void picoxcell_setup_restart(void)
+{
+	struct device_node *np = of_find_compatible_node(NULL, NULL,
+							 "snps,dw-apb-wdg");
+	if (WARN(!np, "unable to setup watchdog restart"))
+		return;
+
+	wdt_regs = of_iomap(np, 0);
+	WARN(!wdt_regs, "failed to remap watchdog regs");
+}
+
 static struct map_desc io_map __initdata = {
 	.virtual	= PHYS_TO_IO(PICOXCELL_PERIPH_BASE),
 	.pfn		= __phys_to_pfn(PICOXCELL_PERIPH_BASE),
@@ -38,6 +55,7 @@ static void __init picoxcell_map_io(void)
 static void __init picoxcell_init_machine(void)
 {
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+	picoxcell_setup_restart();
 }
 
 static const char *picoxcell_dt_match[] = {
@@ -56,6 +74,19 @@ static void __init picoxcell_init_irq(void)
 	of_irq_init(vic_of_match);
 }
 
+static void picoxcell_wdt_restart(char mode, const char *cmd)
+{
+	/*
+	 * Configure the watchdog to reset with the shortest possible timeout
+	 * and give it chance to do the reset.
+	 */
+	if (wdt_regs) {
+		__raw_writel(WDT_CTRL_REG_EN_MASK, wdt_regs + WDT_CTRL_REG_OFFS);
+		__raw_writel(0, wdt_regs + WDT_TIMEOUT_REG_OFFS);
+		mdelay(500);
+	}
+}
+
 DT_MACHINE_START(PICOXCELL, "Picochip picoXcell")
 	.map_io		= picoxcell_map_io,
 	.nr_irqs	= NR_IRQS_LEGACY,
@@ -64,4 +95,5 @@ DT_MACHINE_START(PICOXCELL, "Picochip picoXcell")
 	.timer		= &picoxcell_timer,
 	.init_machine	= picoxcell_init_machine,
 	.dt_compat	= picoxcell_dt_match,
+	.restart	= picoxcell_wdt_restart,
 MACHINE_END
-- 
1.7.5.4

[PATCHv2 5/5] ARM: picoxcell: implement watchdog restart

[Arnd Bergmann]

On Saturday 17 December 2011 13:42:37 Jamie Iles wrote:
> Allow the platform to be restarted by triggering the watchdog to expire
> with the shortest possible expiry.  This should reset the CPU core and
> all on-chip peripherals.
> 
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>

Hi Jamie,

> @@ -23,6 +24,22 @@
>  
>  #include "common.h"
>  
> +#define WDT_CTRL_REG_EN_MASK	(1 << 0)
> +#define WDT_CTRL_REG_OFFS	(0x00)
> +#define WDT_TIMEOUT_REG_OFFS	(0x04)
> +static void __iomem *wdt_regs;
> +
> +static void picoxcell_setup_restart(void)
> +{
> +	struct device_node *np = of_find_compatible_node(NULL, NULL,
> +							 "snps,dw-apb-wdg");
> +	if (WARN(!np, "unable to setup watchdog restart"))
> +		return;
> +
> +	wdt_regs = of_iomap(np, 0);
> +	WARN(!wdt_regs, "failed to remap watchdog regs");
> +}

Can you do this at restart time? I think it would be nicer to
avoid the global variable and init function and instead have
everything in a single function call for restart

> +static void picoxcell_wdt_restart(char mode, const char *cmd)
> +{
> +	/*
> +	 * Configure the watchdog to reset with the shortest possible timeout
> +	 * and give it chance to do the reset.
> +	 */
> +	if (wdt_regs) {
> +		__raw_writel(WDT_CTRL_REG_EN_MASK, wdt_regs + WDT_CTRL_REG_OFFS);
> +		__raw_writel(0, wdt_regs + WDT_TIMEOUT_REG_OFFS);
> +		mdelay(500);
> +	}
> +}

Does this work if the real watchdog is still running, on a preemtible kernel?

Also, please always use writel_relaxed instead of __raw_writel, and try to use
msleep instead of mdelay by default. If msleep doesn't work here because it
actually causes the system not to be reset, just add a comment.

	Arnd

[PATCHv2 0/5] Misc picoxcell updates for 3.3

[Arnd Bergmann]

On Saturday 17 December 2011 13:42:32 Jamie Iles wrote:
> Here's v2 of a small series of feature updates for picoxcell in 3.3.
> 
> Jamie
> 
> Jamie Iles (5):
>   ARM: picoxcell: remove mach/memory.h
>   ARM: picoxcell: don't reserve irq_descs
>   ARM: picoxcell: move io mappings to common.c
>   MAINTAINERS: add maintainer entry for Picochip picoxcell
>   ARM: picoxcell: implement watchdog restart

Hi Jamie,

The series looks good to me, aside from the trivial comments I've given
on patch 5. When you submit these for inclusion in arm-soc, please split
the series so that the first four patches can go into the next/cleanup
branch, while the last patch goes into the devel branch.

	Arnd

[PATCHv2 5/5] ARM: picoxcell: implement watchdog restart

[Jamie Iles]

Hi Arnd,

On Sat, Dec 17, 2011 at 09:17:00PM +0100, Arnd Bergmann wrote:
> On Saturday 17 December 2011 13:42:37 Jamie Iles wrote:
> > Allow the platform to be restarted by triggering the watchdog to expire
> > with the shortest possible expiry.  This should reset the CPU core and
> > all on-chip peripherals.
> > 
> > Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> 
> Hi Jamie,
> 
> > @@ -23,6 +24,22 @@
> >  
> >  #include "common.h"
> >  
> > +#define WDT_CTRL_REG_EN_MASK	(1 << 0)
> > +#define WDT_CTRL_REG_OFFS	(0x00)
> > +#define WDT_TIMEOUT_REG_OFFS	(0x04)
> > +static void __iomem *wdt_regs;
> > +
> > +static void picoxcell_setup_restart(void)
> > +{
> > +	struct device_node *np = of_find_compatible_node(NULL, NULL,
> > +							 "snps,dw-apb-wdg");
> > +	if (WARN(!np, "unable to setup watchdog restart"))
> > +		return;
> > +
> > +	wdt_regs = of_iomap(np, 0);
> > +	WARN(!wdt_regs, "failed to remap watchdog regs");
> > +}
> 
> Can you do this at restart time? I think it would be nicer to
> avoid the global variable and init function and instead have
> everything in a single function call for restart
> 
> > +static void picoxcell_wdt_restart(char mode, const char *cmd)
> > +{
> > +	/*
> > +	 * Configure the watchdog to reset with the shortest possible timeout
> > +	 * and give it chance to do the reset.
> > +	 */
> > +	if (wdt_regs) {
> > +		__raw_writel(WDT_CTRL_REG_EN_MASK, wdt_regs + WDT_CTRL_REG_OFFS);
> > +		__raw_writel(0, wdt_regs + WDT_TIMEOUT_REG_OFFS);
> > +		mdelay(500);
> > +	}
> > +}
> 
> Does this work if the real watchdog is still running, on a preemtible kernel?

Yes, this sets the timeout period to about 3us on this platform so 
there's no way the driver should be pinging it that fast even if it was 
still active.

> Also, please always use writel_relaxed instead of __raw_writel, and try to use
> msleep instead of mdelay by default. If msleep doesn't work here because it
> actually causes the system not to be reset, just add a comment.

OK fair point.  I was a bit wary of doing too much in the actual reboot 
time as it wasn't clear what services are available at that time, but 
all of your changes work so I'll go ahead and implement them.

Thanks,

Jamie

[PATCHv2 5/5] ARM: picoxcell: implement watchdog restart

[Russell King - ARM Linux]

On Sat, Dec 17, 2011 at 09:17:00PM +0100, Arnd Bergmann wrote:
> > +static void picoxcell_wdt_restart(char mode, const char *cmd)
> > +{
> > +	/*
> > +	 * Configure the watchdog to reset with the shortest possible timeout
> > +	 * and give it chance to do the reset.
> > +	 */
> > +	if (wdt_regs) {
> > +		__raw_writel(WDT_CTRL_REG_EN_MASK, wdt_regs + WDT_CTRL_REG_OFFS);
> > +		__raw_writel(0, wdt_regs + WDT_TIMEOUT_REG_OFFS);
> > +		mdelay(500);
> > +	}
> > +}
> 
> Does this work if the real watchdog is still running, on a preemtible kernel?
> 
> Also, please always use writel_relaxed instead of __raw_writel, and try to use
> msleep instead of mdelay by default. If msleep doesn't work here because it
> actually causes the system not to be reset, just add a comment.

Restarting can be called from atomic contexts, so any function which may
sleep must not be used.  That include sioremap.

[PATCHv2 5/5] ARM: picoxcell: implement watchdog restart

[Jamie Iles]

On Sun, Dec 18, 2011 at 08:13:27AM +0000, Russell King - ARM Linux wrote:
> On Sat, Dec 17, 2011 at 09:17:00PM +0100, Arnd Bergmann wrote:
> > > +static void picoxcell_wdt_restart(char mode, const char *cmd)
> > > +{
> > > +	/*
> > > +	 * Configure the watchdog to reset with the shortest possible timeout
> > > +	 * and give it chance to do the reset.
> > > +	 */
> > > +	if (wdt_regs) {
> > > +		__raw_writel(WDT_CTRL_REG_EN_MASK, wdt_regs + WDT_CTRL_REG_OFFS);
> > > +		__raw_writel(0, wdt_regs + WDT_TIMEOUT_REG_OFFS);
> > > +		mdelay(500);
> > > +	}
> > > +}
> > 
> > Does this work if the real watchdog is still running, on a preemtible kernel?
> > 
> > Also, please always use writel_relaxed instead of __raw_writel, and try to use
> > msleep instead of mdelay by default. If msleep doesn't work here because it
> > actually causes the system not to be reset, just add a comment.
> 
> Restarting can be called from atomic contexts, so any function which may
> sleep must not be used.  That include sioremap.

Thanks for clarifying Russell.  I'll add a comment or two to that effect 
and change the __raw_writel's to writel_relaxed().

Thanks,

Jamie