Patchwork [RFC] ARM: new architecture for Energy Micro's EFM32 Cortex-M3 SoCs

login
register
mail settings
Submitter Uwe Kleine-König
Date Dec. 21, 2011, 3:13 p.m.
Message ID <1324480428-13344-1-git-send-email-u.kleine-koenig@pengutronix.de>
Download mbox | patch
Permalink /patch/132673/
State New
Headers show

Comments

Uwe Kleine-König - Dec. 21, 2011, 3:13 p.m.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

currently this relies on the bootloader to enable all necessary clocks
because there is no functional clk API, yet.

Other than that my machine comes up, but the 1 MiB of RAM is very tight,
so I have to stick to XIP.

Also note that this depends on ARMv7M support that is picked from
Catalin's repository. I didn't come around yet to rework them into a
shape acceptable for mainline. I ported them to 3.2-rc and needed a few
more hacks (e.g. I don't have a bootloader, so I added a little
assembler bootloader that is included at the start of my xipImage).
If you're interested in the details I can publish my tree I think.

Best regards
Uwe

 arch/arm/Kconfig                               |   11 ++-
 arch/arm/Kconfig.debug                         |    8 +
 arch/arm/Makefile                              |    1 +
 arch/arm/boot/dts/efm32gg-dk3750.dts           |   41 ++++++
 arch/arm/mach-efm32/Makefile                   |    3 +
 arch/arm/mach-efm32/Makefile.boot              |    1 +
 arch/arm/mach-efm32/clk.c                      |   30 +++++
 arch/arm/mach-efm32/common.h                   |    7 +
 arch/arm/mach-efm32/dtmachine.c                |   47 +++++++
 arch/arm/mach-efm32/include/mach/debug-macro.S |   32 +++++
 arch/arm/mach-efm32/include/mach/entry-macro.S |   12 ++
 arch/arm/mach-efm32/include/mach/io.h          |    7 +
 arch/arm/mach-efm32/include/mach/irqs.h        |    6 +
 arch/arm/mach-efm32/include/mach/system.h      |   18 +++
 arch/arm/mach-efm32/include/mach/timex.h       |    7 +
 arch/arm/mach-efm32/time.c                     |  166 ++++++++++++++++++++++++
 arch/arm/mm/Kconfig                            |    2 +-
 17 files changed, 397 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/boot/dts/efm32gg-dk3750.dts
 create mode 100644 arch/arm/mach-efm32/Makefile
 create mode 100644 arch/arm/mach-efm32/Makefile.boot
 create mode 100644 arch/arm/mach-efm32/clk.c
 create mode 100644 arch/arm/mach-efm32/common.h
 create mode 100644 arch/arm/mach-efm32/dtmachine.c
 create mode 100644 arch/arm/mach-efm32/include/mach/debug-macro.S
 create mode 100644 arch/arm/mach-efm32/include/mach/entry-macro.S
 create mode 100644 arch/arm/mach-efm32/include/mach/io.h
 create mode 100644 arch/arm/mach-efm32/include/mach/irqs.h
 create mode 100644 arch/arm/mach-efm32/include/mach/system.h
 create mode 100644 arch/arm/mach-efm32/include/mach/timex.h
 create mode 100644 arch/arm/mach-efm32/time.c
Jamie Iles - Dec. 21, 2011, 3:48 p.m.
Hi Uwe,

Just nits inline, but otherwise looks nice!  Certainly seems like an 
interesting platform!

Jamie

On Wed, Dec 21, 2011 at 04:13:48PM +0100, Uwe Kleine-König wrote:
[...]
> diff --git a/arch/arm/boot/dts/efm32gg-dk3750.dts b/arch/arm/boot/dts/efm32gg-dk3750.dts
> new file mode 100644
> index 0000000..a4aa4f3
> --- /dev/null
> +++ b/arch/arm/boot/dts/efm32gg-dk3750.dts
> @@ -0,0 +1,41 @@
> +/dts-v1/;
> +/include/ "skeleton.dtsi"
> +
> +/ {
> +	model = "Energy Micro Giant Gecko Development Kit";
> +	compatible = "efm32,dk3750";
> +
> +	aliases {
> +		serial1 = &uart1;
> +	};
> +
> +	nvic: nv-interrupt-controller@0xe0000000  {
> +		compatible = "arm,cortex-m3-nvic";
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +		reg = <0xe0000000 0x4000>;
> +	};

I guess there's some patches somewhere else for the nvic binding, but I 
thought that the nvic had a reasonable number of features like 
edge/level etc and so would have more than one intererupt cell...

> +	chosen {
> +		bootargs = "console=ttyefm1,115200 init=/linuxrc ignore_loglevel ihash_entries=64 dhash_entries=64";
> +	};
> +
> +	memory {

I think "memory@80000000" is preferred here.

> +		reg = <0x80000000 0x100000>;
> +	};
> +
> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";
> +		interrupt-parent = <&nvic>;
> +		ranges;
> +
> +		uart1: uart@0x4000c400 { /* USART1 */
> +			compatible = "efm32,usart";
> +			reg = <0x4000c400 0x400>;
> +			interrupts = <15>;
> +			status = "ok";
> +		};
> +	};
> +};
> diff --git a/arch/arm/mach-efm32/common.h 
> b/arch/arm/mach-efm32/common.h
> new file mode 100644
> index 0000000..f545224
> --- /dev/null
> +++ b/arch/arm/mach-efm32/common.h
> @@ -0,0 +1,7 @@
> +#ifdef __ASSEMBLER__
> +#define IOMEM(addr)     (addr)
> +#else
> +#define IOMEM(addr)     ((void __force __iomem *)(addr))
> +#endif

It looks like the only users of IOMEM are for the timers.  Is it 
possible to get the timer information from the device tree then remove 
this?

> +extern struct sys_timer efm32_timer;
> diff --git a/arch/arm/mach-efm32/dtmachine.c b/arch/arm/mach-efm32/dtmachine.c
> new file mode 100644
> index 0000000..42d091c
> --- /dev/null
> +++ b/arch/arm/mach-efm32/dtmachine.c
> @@ -0,0 +1,47 @@
> +#include <linux/kernel.h>
> +#include <linux/pinctrl/machine.h>
> +#include <linux/irqdomain.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_irq.h>
> +
> +#include <asm/hardware/nvic.h>
> +#include <asm/mach/arch.h>
> +#include <asm/mach/time.h>
> +
> +#include "common.h"
> +
> +static void __init efm32_nvic_add_irq_domain(struct device_node *np,
> +		struct device_node *interrupt_parent)
> +{
> +	irq_domain_add_simple(np, 0);
> +}

If the nvic driver is new then I would have expected that to have device 
tree and irq domain support internally.  From the entry macros it looks 
like you're using the GIC macros, so is it pretty much GIC compatible?

> +
> +static const struct of_device_id efm32_irq_match[] __initconst = {
> +	{
> +		.compatible = "arm,cortex-m3-nvic",
> +		.data = efm32_nvic_add_irq_domain,
> +	}, {
> +		/* sentinel */
> +	}
> +};
> +
> +static void __init efm32_init(void)
> +{
> +	int ret;

This doesn't appear to be used.

> +
> +	of_irq_init(efm32_irq_match);

Shouldn't this be in a .init_irq function()?

> +
> +	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> +}
> +
> +static const char *const efm32gg_compat[] __initconst = {
> +	"efm32,dk3750",
> +	NULL
> +};
> +
> +DT_MACHINE_START(EFM32DT, "EFM32 (Device Tree Support)")
> +	.init_irq = nvic_init,

This doesn't exist...

> +	.timer = &efm32_timer,
> +	.init_machine = efm32_init,
> +	.dt_compat = efm32gg_compat,
> +MACHINE_END
> diff --git a/arch/arm/mach-efm32/include/mach/io.h 
> b/arch/arm/mach-efm32/include/mach/io.h
> new file mode 100644
> index 0000000..00b6af6
> --- /dev/null
> +++ b/arch/arm/mach-efm32/include/mach/io.h
> @@ -0,0 +1,7 @@
> +#ifndef __MACH_IO_H__
> +#define __MACH_IO_H__
> +
> +#define __io(a)			__typesafe_io(a)

Do you support io ports on this platform?  If not then perhaps select 
NO_IPORT and define __io(a) to 0?

> +#define __mem_pci(a)		(a)
> +
> +#endif /* __MACH_IO_H__ */
> diff --git a/arch/arm/mach-efm32/include/mach/irqs.h b/arch/arm/mach-efm32/include/mach/irqs.h
> new file mode 100644
> index 0000000..5fa84db
> --- /dev/null
> +++ b/arch/arm/mach-efm32/include/mach/irqs.h
> @@ -0,0 +1,6 @@
> +#ifndef __MACH_IRQS_H__
> +#define __MACH_IRQS_H__
> +
> +#define NR_IRQS			37

Is it possible to use sparse irq and have the nvic driver allocate the 
irq descs?  I guess multi-platform is less likely on platforms with 
small amounts of RAM though...

> +
> +#endif /* __MACH_IRQS_H__ */
> diff --git a/arch/arm/mach-efm32/include/mach/system.h b/arch/arm/mach-efm32/include/mach/system.h
> new file mode 100644
> index 0000000..619222c
> --- /dev/null
> +++ b/arch/arm/mach-efm32/include/mach/system.h
> @@ -0,0 +1,18 @@
> +#ifndef __MACH_SYSTEM_H__
> +#define __MACH_SYSTEM_H__
> +
> +#include <asm/io.h>
> +
> +static inline void arch_idle(void)
> +{
> +	cpu_do_idle();
> +}
> +
> +static inline void arch_reset(char mode, const char *cmd)
> +{
> +	/* XXX: move this to (say) cpuv7m_reset */
> +	dsb();
> +	__raw_writel(0x05fa0004, (void __iomem *)0xe000ed0c);
> +	dsb();
> +}
> +#endif /* __MACH_SYSTEM_H__ */

I guess this would need to be rebased on Russell and Nicolas' series' 
that remove arch_idle() and arch_reset() from here and moves the reset 
into the common.c portion and part of the machine desc.

> diff --git a/arch/arm/mach-efm32/time.c b/arch/arm/mach-efm32/time.c
> new file mode 100644
> index 0000000..65f93a3
> --- /dev/null
> +++ b/arch/arm/mach-efm32/time.c
> @@ -0,0 +1,166 @@
> +#include <linux/kernel.h>
> +#include <linux/clocksource.h>
> +#include <linux/clockchips.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/stringify.h>
> +
> +#include <asm/mach/time.h>
> +
> +#include <common.h>

Perhaps:

#include "common.h"

to make it clear it's a local include?

> +
> +#define BASEADDR_TIMER(n)		IOMEM(0x40010000 + (n) * 0x400)

Is it not possible to get this from the device tree rather than hard 
coding it?

> +
> +#define TIMERn_CTRL			0x00
> +#define TIMERn_CTRL_PRESC(val)			(((val) & 0xf) << 24)
> +#define TIMERn_CTRL_PRESC_1024			TIMERn_CTRL_PRESC(10)
> +#define TIMERn_CTRL_CLKSEL(val)			(((val) & 0x3) << 16)
> +#define TIMERn_CTRL_CLKSEL_PRESCHFPERCLK	TIMERn_CTRL_CLKSEL(0)
> +#define TIMERn_CTRL_OSMEN			0x00000010
> +#define TIMERn_CTRL_MODE(val)			(((val) & 0x3) <<  0)
> +#define TIMERn_CTRL_MODE_UP			TIMERn_CTRL_MODE(0)
> +#define TIMERn_CTRL_MODE_DOWN			TIMERn_CTRL_MODE(1)
> +
> +#define TIMERn_CMD			0x04
> +#define TIMERn_CMD_START			0x1
> +#define TIMERn_CMD_STOP				0x2
> +
> +#define TIMERn_IEN			0x0c
> +#define TIMERn_IF			0x10
> +#define TIMERn_IFS			0x14
> +#define TIMERn_IFC			0x18
> +#define TIMERn_IRQ_UF				0x2
> +#define TIMERn_IRQ_OF				0x1
> +
> +#define TIMERn_TOP			0x1c
> +#define TIMERn_CNT			0x24
> +
> +#define TIMER_CLOCKSOURCE	1
> +#define TIMER_CLOCKEVENT	2
> +#define IRQ_CLOCKEVENT		13
> +
> +static void efm32_timer_write(unsigned timerno, u32 val, unsigned offset)
> +{
> +	__raw_writel(val, BASEADDR_TIMER(timerno) + offset);

I believe writel_relaxed() is preferred where possible over 
__raw_writel().

> +}
> +
> +static void efm32_clock_event_set_mode(enum clock_event_mode mode,
> +		struct clock_event_device *unused)
> +{
> +	switch (mode) {
> +	case CLOCK_EVT_MODE_PERIODIC:
> +		efm32_timer_write(TIMER_CLOCKEVENT,
> +				TIMERn_CMD_STOP, TIMERn_CMD);
> +		efm32_timer_write(TIMER_CLOCKEVENT, 137, TIMERn_TOP);
> +		efm32_timer_write(TIMER_CLOCKEVENT,
> +				TIMERn_CTRL_PRESC_1024 |
> +				TIMERn_CTRL_CLKSEL_PRESCHFPERCLK |
> +				TIMERn_CTRL_MODE_DOWN, TIMERn_CTRL);
> +		efm32_timer_write(TIMER_CLOCKEVENT,
> +				TIMERn_CMD_START, TIMERn_CMD);
> +		break;
> +
> +	case CLOCK_EVT_MODE_ONESHOT:
> +		efm32_timer_write(TIMER_CLOCKEVENT,
> +				TIMERn_CMD_STOP, TIMERn_CMD);
> +		efm32_timer_write(TIMER_CLOCKEVENT,
> +				TIMERn_CTRL_PRESC_1024 |
> +				TIMERn_CTRL_CLKSEL_PRESCHFPERCLK |
> +				TIMERn_CTRL_OSMEN |
> +				TIMERn_CTRL_MODE_DOWN, TIMERn_CTRL);
> +		break;
> +
> +	case CLOCK_EVT_MODE_UNUSED:
> +	case CLOCK_EVT_MODE_SHUTDOWN:
> +		efm32_timer_write(TIMER_CLOCKEVENT, TIMERn_CMD_STOP,
> +				TIMERn_CMD);
> +		break;
> +
> +	case CLOCK_EVT_MODE_RESUME:
> +		break;
> +	}
> +}
> +
> +static int efm32_clock_event_set_next_event(unsigned long evt,
> +		struct clock_event_device *unused)
> +{
> +	/* writing START to CMD restarts a running timer, too */
> +	efm32_timer_write(TIMER_CLOCKEVENT, evt, TIMERn_TOP);
> +	efm32_timer_write(TIMER_CLOCKEVENT, TIMERn_CMD_START, TIMERn_CMD);
> +
> +	return 0;
> +}
> +
> +static struct clock_event_device efm32_clock_event_device = {
> +	.name = "efm32 clockevent (" __stringify(TIMER_CLOCKEVENT) ")",
> +	.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_MODE_PERIODIC,
> +	.set_mode = efm32_clock_event_set_mode,
> +	.set_next_event = efm32_clock_event_set_next_event,
> +	.rating = 200,
> +};
> +
> +static irqreturn_t efm32_clock_event_handler(int irq, void *dev_id)
> +{
> +	struct clock_event_device *evt = &efm32_clock_event_device;
> +
> +	/* ack irq */
> +	efm32_timer_write(TIMER_CLOCKEVENT, TIMERn_IRQ_UF, TIMERn_IFC);
> +
> +	evt->event_handler(evt);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static struct irqaction efm32_clock_event_irq = {
> +	.name = "efm32 clockevent",
> +	.flags = IRQF_TIMER,
> +	.handler = efm32_clock_event_handler,
> +	.dev_id = &efm32_clock_event_device,
> +};
> +
> +/*
> + * XXX: use clk_ API to get frequency and enabling of the clocks used.
> + * Here the reset defaults are used:
> + * - freq_{HFPERCLK} = freq_{HFCLK}
> + *   (CMU_HFPERCLKDIV_HFPERCLKDIV = 0x0)
> + * - freq_{HFCLK} = freq_{HFRCO}
> + *   (CMU_CTRL_HFCLKDIV = 0x0, CMU_STATUS_HFRCOSEL = 0x1)
> + * - freq_{HFRCO} = 14MHz
> + *   (CMU_HFRCOCTRL_BAND = 0x3)
> + *
> + * So the HFPERCLK runs at 14MHz. The timer has an additional prescaler
> + * programmed to /1024. This make the timer run at
> + *
> + * 	14 MHz / 1024 = 13671.875 Hz
> + *
> + */
> +static void __init efm32_timer_init(void)
> +{
> +	/* enable CMU_HFPERCLKEN0_TIMERn for clocksource via bit-band */
> +	__raw_writel(1, IOMEM(0x43900894 + 4 * TIMER_CLOCKSOURCE));
> +
> +	efm32_timer_write(TIMER_CLOCKSOURCE,
> +			TIMERn_CTRL_PRESC_1024 |
> +			TIMERn_CTRL_CLKSEL_PRESCHFPERCLK |
> +			TIMERn_CTRL_MODE_UP, TIMERn_CTRL);
> +	efm32_timer_write(TIMER_CLOCKSOURCE, TIMERn_CMD_START, TIMERn_CMD);
> +
> +	clocksource_mmio_init(BASEADDR_TIMER(TIMER_CLOCKSOURCE) + TIMERn_CNT,
> +			"efm32 timer", 13672, 200, 16,
> +			clocksource_mmio_readl_up);
> +
> +	/* enable CMU_HFPERCLKEN0_TIMERn for clockevent via bit-band */
> +	__raw_writel(1, IOMEM(0x43900894 + 4 * TIMER_CLOCKEVENT));
> +
> +	efm32_timer_write(TIMER_CLOCKEVENT, TIMERn_IRQ_UF, TIMERn_IEN);
> +
> +	setup_irq(IRQ_CLOCKEVENT, &efm32_clock_event_irq);

Do you need to use setup_irq() here?  For picoxcell I used device tree 
and so irq_of_parse_and_map() but that seemed fine for getting a timer 
up and running.

Jamie
Arnd Bergmann - Dec. 21, 2011, 4:34 p.m.
On Wednesday 21 December 2011, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> currently this relies on the bootloader to enable all necessary clocks
> because there is no functional clk API, yet.
> 
> Other than that my machine comes up, but the 1 MiB of RAM is very tight,
> so I have to stick to XIP.
> 
> Also note that this depends on ARMv7M support that is picked from
> Catalin's repository. I didn't come around yet to rework them into a
> shape acceptable for mainline. I ported them to 3.2-rc and needed a few
> more hacks (e.g. I don't have a bootloader, so I added a little
> assembler bootloader that is included at the start of my xipImage).
> If you're interested in the details I can publish my tree I think.

Hi Uwe,

It's great to see you managed to get the port done using the device tree.
You mentioned earlier that you were worried about the object size overhead
in doing so. Do you have any numbers already about the actual memory
consumption of the DT based port compared to your earlier one?

> +	chosen {
> +		bootargs = "console=ttyefm1,115200 init=/linuxrc ignore_loglevel ihash_entries=64 dhash_entries=64";
> +	};

I don't know how we do this on other boards, but is it intentional to 
leave the bootargs in the device tree source? I would expect that they
normally would have to be changed by the boot loader.

> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";
> +		interrupt-parent = <&nvic>;
> +		ranges;
> +
> +		uart1: uart@0x4000c400 { /* USART1 */
> +			compatible = "efm32,usart";
> +			reg = <0x4000c400 0x400>;
> +			interrupts = <15>;
> +			status = "ok";
> +		};
> +	};

This will have to grow to include the other devices for inclusion, right?

> diff --git a/arch/arm/mach-efm32/Makefile b/arch/arm/mach-efm32/Makefile
> new file mode 100644
> index 0000000..10a3426
> --- /dev/null
> +++ b/arch/arm/mach-efm32/Makefile
> @@ -0,0 +1,3 @@
> +obj-y += clk.o time.o
> +
> +obj-$(CONFIG_OF) += dtmachine.o

I think you can rename the dtmachine.c file into common.c or similar and build
it unconditionally, as we do on the other new platforms.

> diff --git a/arch/arm/mach-efm32/include/mach/io.h b/arch/arm/mach-efm32/include/mach/io.h
> new file mode 100644
> index 0000000..00b6af6
> --- /dev/null
> +++ b/arch/arm/mach-efm32/include/mach/io.h
> @@ -0,0 +1,7 @@
> +#ifndef __MACH_IO_H__
> +#define __MACH_IO_H__
> +
> +#define __io(a)			__typesafe_io(a)
> +#define __mem_pci(a)		(a)
> +
> +#endif /* __MACH_IO_H__ */

Can you already build with __io not defined? That should catch any drivers
that try to access the PC-style I/O ports.

> +static inline void arch_reset(char mode, const char *cmd)
> +{
> +	/* XXX: move this to (say) cpuv7m_reset */
> +	dsb();
> +	__raw_writel(0x05fa0004, (void __iomem *)0xe000ed0c);
> +	dsb();
> +}

I'd like to hear other opinions on this one. It's clearly an ugly hack
to just poke a random MMIO register for reset, but we could decide
that it's useful enough to keep the implementation just like this
because a cleaner implementation using a device_node and io_iomap
just causes overhead that you don't want to spend a system that
is so limited on memory.

As Jamie mentioned, using writel_relaxed() would be better here
and cause no overhead.

> +/* just a bogus value */
> +#define CLOCK_TICK_RATE	12345678

I think you should actually pick a number that results in no rounding
in the jiffies code. 1000000 would be a reasonable choice, as would
any other multiple of HZ.

> diff --git a/arch/arm/mach-efm32/time.c b/arch/arm/mach-efm32/time.c
> new file mode 100644
> index 0000000..65f93a3
> --- /dev/null
> +++ b/arch/arm/mach-efm32/time.c

I think Jamie already said everything necessary about this file.

> --- a/arch/arm/mm/Kconfig
> +++ b/arch/arm/mm/Kconfig
> @@ -432,7 +432,7 @@ config CPU_V7
>  # ARMv7
>  config CPU_V7M
>  	bool "Support ARMv7-M processors"
> -	depends on MACH_REALVIEW_EB && EXPERIMENTAL
> +	depends on (MACH_REALVIEW_EB && EXPERIMENTAL) || ARCH_EFM32
>  	select THUMB2_KERNEL
>  	select ARM_THUMB
>  	select CPU_32v7M

Hmm, since you *have* to enable this option for a successful build, wouldn't it
make more sense to 'select' this from ARCH_EFM32.

	Arnd
Uwe Kleine-König - Dec. 21, 2011, 4:44 p.m.
Hi Jamie,

On Wed, Dec 21, 2011 at 03:48:50PM +0000, Jamie Iles wrote:
> Just nits inline, but otherwise looks nice!  Certainly seems like an 
> interesting platform!
Yes it is. I hope to get the silicon version of it in January, this will
have 4 MiB of RAM and so might be better suited for Linux. (Currently
when booting with dt I get a prompt but starting a program makes the
kernel BUG because there isn't enough memory.)

> On Wed, Dec 21, 2011 at 04:13:48PM +0100, Uwe Kleine-König wrote:
> [...]
> > diff --git a/arch/arm/boot/dts/efm32gg-dk3750.dts b/arch/arm/boot/dts/efm32gg-dk3750.dts
> > new file mode 100644
> > index 0000000..a4aa4f3
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/efm32gg-dk3750.dts
> > @@ -0,0 +1,41 @@
> > +/dts-v1/;
> > +/include/ "skeleton.dtsi"
> > +
> > +/ {
> > +	model = "Energy Micro Giant Gecko Development Kit";
> > +	compatible = "efm32,dk3750";
> > +
> > +	aliases {
> > +		serial1 = &uart1;
> > +	};
> > +
> > +	nvic: nv-interrupt-controller@0xe0000000  {
> > +		compatible = "arm,cortex-m3-nvic";
> > +		interrupt-controller;
> > +		#interrupt-cells = <1>;
> > +		reg = <0xe0000000 0x4000>;
> > +	};
> 
> I guess there's some patches somewhere else for the nvic binding, but I 
> thought that the nvic had a reasonable number of features like 
> edge/level etc and so would have more than one intererupt cell...
This dt stuff is all quite new to me. I will check what needs to be done
here. Support for the nvic is part of Catalin's patches that need some
more love to be included in mainline.
 
> > +	chosen {
> > +		bootargs = "console=ttyefm1,115200 init=/linuxrc ignore_loglevel ihash_entries=64 dhash_entries=64";
> > +	};
> > +
> > +	memory {
> 
> I think "memory@80000000" is preferred here.
ok.

> > +		reg = <0x80000000 0x100000>;
> > +	};
> > +
> > +	soc {
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		compatible = "simple-bus";
> > +		interrupt-parent = <&nvic>;
> > +		ranges;
> > +
> > +		uart1: uart@0x4000c400 { /* USART1 */
> > +			compatible = "efm32,usart";
> > +			reg = <0x4000c400 0x400>;
> > +			interrupts = <15>;
> > +			status = "ok";
> > +		};
> > +	};
> > +};
> > diff --git a/arch/arm/mach-efm32/common.h 
> > b/arch/arm/mach-efm32/common.h
> > new file mode 100644
> > index 0000000..f545224
> > --- /dev/null
> > +++ b/arch/arm/mach-efm32/common.h
> > @@ -0,0 +1,7 @@
> > +#ifdef __ASSEMBLER__
> > +#define IOMEM(addr)     (addr)
> > +#else
> > +#define IOMEM(addr)     ((void __force __iomem *)(addr))
> > +#endif
> 
> It looks like the only users of IOMEM are for the timers.  Is it 
I still wait for this to be moved to generic ARM code.

> possible to get the timer information from the device tree then remove 
> this?
Probably yes. Though I'll have to wait for the bigger machine to test
putting more into dt.
 
> > +extern struct sys_timer efm32_timer;
> > diff --git a/arch/arm/mach-efm32/dtmachine.c b/arch/arm/mach-efm32/dtmachine.c
> > new file mode 100644
> > index 0000000..42d091c
> > --- /dev/null
> > +++ b/arch/arm/mach-efm32/dtmachine.c
> > @@ -0,0 +1,47 @@
> > +#include <linux/kernel.h>
> > +#include <linux/pinctrl/machine.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/of_irq.h>
> > +
> > +#include <asm/hardware/nvic.h>
> > +#include <asm/mach/arch.h>
> > +#include <asm/mach/time.h>
> > +
> > +#include "common.h"
> > +
> > +static void __init efm32_nvic_add_irq_domain(struct device_node *np,
> > +		struct device_node *interrupt_parent)
> > +{
> > +	irq_domain_add_simple(np, 0);
> > +}
> 
> If the nvic driver is new then I would have expected that to have device 
> tree and irq domain support internally.  From the entry macros it looks 
> like you're using the GIC macros, so is it pretty much GIC compatible?
Yeah, the relevant register offsets are identical. I think the nvic has
less power management capabilities and supports less irqs. I will
address this when I rework the nvic patch.

> > +
> > +static const struct of_device_id efm32_irq_match[] __initconst = {
> > +	{
> > +		.compatible = "arm,cortex-m3-nvic",
> > +		.data = efm32_nvic_add_irq_domain,
> > +	}, {
> > +		/* sentinel */
> > +	}
> > +};
> > +
> > +static void __init efm32_init(void)
> > +{
> > +	int ret;
> 
> This doesn't appear to be used.
yeah, that's only used in my tree that has a few more in the init
routine.
 
> > +
> > +	of_irq_init(efm32_irq_match);
> 
> Shouldn't this be in a .init_irq function()?
I think I copied that from imx. So if you're right imx needs fixing,
too.

> > +
> > +	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> > +}
> > +
> > +static const char *const efm32gg_compat[] __initconst = {
> > +	"efm32,dk3750",
> > +	NULL
> > +};
> > +
> > +DT_MACHINE_START(EFM32DT, "EFM32 (Device Tree Support)")
> > +	.init_irq = nvic_init,
> 
> This doesn't exist...
yeah, see above.
 
> > +	.timer = &efm32_timer,
> > +	.init_machine = efm32_init,
> > +	.dt_compat = efm32gg_compat,
> > +MACHINE_END
> > diff --git a/arch/arm/mach-efm32/include/mach/io.h 
> > b/arch/arm/mach-efm32/include/mach/io.h
> > new file mode 100644
> > index 0000000..00b6af6
> > --- /dev/null
> > +++ b/arch/arm/mach-efm32/include/mach/io.h
> > @@ -0,0 +1,7 @@
> > +#ifndef __MACH_IO_H__
> > +#define __MACH_IO_H__
> > +
> > +#define __io(a)			__typesafe_io(a)
> 
> Do you support io ports on this platform?  If not then perhaps select 
> NO_IPORT and define __io(a) to 0?
> 
> > +#define __mem_pci(a)		(a)
> > +
> > +#endif /* __MACH_IO_H__ */
> > diff --git a/arch/arm/mach-efm32/include/mach/irqs.h b/arch/arm/mach-efm32/include/mach/irqs.h
> > new file mode 100644
> > index 0000000..5fa84db
> > --- /dev/null
> > +++ b/arch/arm/mach-efm32/include/mach/irqs.h
> > @@ -0,0 +1,6 @@
> > +#ifndef __MACH_IRQS_H__
> > +#define __MACH_IRQS_H__
> > +
> > +#define NR_IRQS			37
> 
> Is it possible to use sparse irq and have the nvic driver allocate the 
> irq descs?  I guess multi-platform is less likely on platforms with 
> small amounts of RAM though...
> 
> > +
> > +#endif /* __MACH_IRQS_H__ */
> > diff --git a/arch/arm/mach-efm32/include/mach/system.h b/arch/arm/mach-efm32/include/mach/system.h
> > new file mode 100644
> > index 0000000..619222c
> > --- /dev/null
> > +++ b/arch/arm/mach-efm32/include/mach/system.h
> > @@ -0,0 +1,18 @@
> > +#ifndef __MACH_SYSTEM_H__
> > +#define __MACH_SYSTEM_H__
> > +
> > +#include <asm/io.h>
> > +
> > +static inline void arch_idle(void)
> > +{
> > +	cpu_do_idle();
> > +}
> > +
> > +static inline void arch_reset(char mode, const char *cmd)
> > +{
> > +	/* XXX: move this to (say) cpuv7m_reset */
> > +	dsb();
> > +	__raw_writel(0x05fa0004, (void __iomem *)0xe000ed0c);
> > +	dsb();
> > +}
> > +#endif /* __MACH_SYSTEM_H__ */
> 
> I guess this would need to be rebased on Russell and Nicolas' series' 
> that remove arch_idle() and arch_reset() from here and moves the reset 
> into the common.c portion and part of the machine desc.
Yeah, I'm aware of these changes.

> > diff --git a/arch/arm/mach-efm32/time.c b/arch/arm/mach-efm32/time.c
> > new file mode 100644
> > index 0000000..65f93a3
> > --- /dev/null
> > +++ b/arch/arm/mach-efm32/time.c
> > @@ -0,0 +1,166 @@
> > +#include <linux/kernel.h>
> > +#include <linux/clocksource.h>
> > +#include <linux/clockchips.h>
> > +#include <linux/irq.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/stringify.h>
> > +
> > +#include <asm/mach/time.h>
> > +
> > +#include <common.h>
> 
> Perhaps:
> 
> #include "common.h"
> 
> to make it clear it's a local include?
good catch.

> 
> > +
> > +#define BASEADDR_TIMER(n)		IOMEM(0x40010000 + (n) * 0x400)
> 
> Is it not possible to get this from the device tree rather than hard 
> coding it?
> 
> > +
> > +#define TIMERn_CTRL			0x00
> > +#define TIMERn_CTRL_PRESC(val)			(((val) & 0xf) << 24)
> > +#define TIMERn_CTRL_PRESC_1024			TIMERn_CTRL_PRESC(10)
> > +#define TIMERn_CTRL_CLKSEL(val)			(((val) & 0x3) << 16)
> > +#define TIMERn_CTRL_CLKSEL_PRESCHFPERCLK	TIMERn_CTRL_CLKSEL(0)
> > +#define TIMERn_CTRL_OSMEN			0x00000010
> > +#define TIMERn_CTRL_MODE(val)			(((val) & 0x3) <<  0)
> > +#define TIMERn_CTRL_MODE_UP			TIMERn_CTRL_MODE(0)
> > +#define TIMERn_CTRL_MODE_DOWN			TIMERn_CTRL_MODE(1)
> > +
> > +#define TIMERn_CMD			0x04
> > +#define TIMERn_CMD_START			0x1
> > +#define TIMERn_CMD_STOP				0x2
> > +
> > +#define TIMERn_IEN			0x0c
> > +#define TIMERn_IF			0x10
> > +#define TIMERn_IFS			0x14
> > +#define TIMERn_IFC			0x18
> > +#define TIMERn_IRQ_UF				0x2
> > +#define TIMERn_IRQ_OF				0x1
> > +
> > +#define TIMERn_TOP			0x1c
> > +#define TIMERn_CNT			0x24
> > +
> > +#define TIMER_CLOCKSOURCE	1
> > +#define TIMER_CLOCKEVENT	2
> > +#define IRQ_CLOCKEVENT		13
> > +
> > +static void efm32_timer_write(unsigned timerno, u32 val, unsigned offset)
> > +{
> > +	__raw_writel(val, BASEADDR_TIMER(timerno) + offset);
> 
> I believe writel_relaxed() is preferred where possible over 
> __raw_writel().
> 
> > +}
> > +
> > +static void efm32_clock_event_set_mode(enum clock_event_mode mode,
> > +		struct clock_event_device *unused)
> > +{
> > +	switch (mode) {
> > +	case CLOCK_EVT_MODE_PERIODIC:
> > +		efm32_timer_write(TIMER_CLOCKEVENT,
> > +				TIMERn_CMD_STOP, TIMERn_CMD);
> > +		efm32_timer_write(TIMER_CLOCKEVENT, 137, TIMERn_TOP);
> > +		efm32_timer_write(TIMER_CLOCKEVENT,
> > +				TIMERn_CTRL_PRESC_1024 |
> > +				TIMERn_CTRL_CLKSEL_PRESCHFPERCLK |
> > +				TIMERn_CTRL_MODE_DOWN, TIMERn_CTRL);
> > +		efm32_timer_write(TIMER_CLOCKEVENT,
> > +				TIMERn_CMD_START, TIMERn_CMD);
> > +		break;
> > +
> > +	case CLOCK_EVT_MODE_ONESHOT:
> > +		efm32_timer_write(TIMER_CLOCKEVENT,
> > +				TIMERn_CMD_STOP, TIMERn_CMD);
> > +		efm32_timer_write(TIMER_CLOCKEVENT,
> > +				TIMERn_CTRL_PRESC_1024 |
> > +				TIMERn_CTRL_CLKSEL_PRESCHFPERCLK |
> > +				TIMERn_CTRL_OSMEN |
> > +				TIMERn_CTRL_MODE_DOWN, TIMERn_CTRL);
> > +		break;
> > +
> > +	case CLOCK_EVT_MODE_UNUSED:
> > +	case CLOCK_EVT_MODE_SHUTDOWN:
> > +		efm32_timer_write(TIMER_CLOCKEVENT, TIMERn_CMD_STOP,
> > +				TIMERn_CMD);
> > +		break;
> > +
> > +	case CLOCK_EVT_MODE_RESUME:
> > +		break;
> > +	}
> > +}
> > +
> > +static int efm32_clock_event_set_next_event(unsigned long evt,
> > +		struct clock_event_device *unused)
> > +{
> > +	/* writing START to CMD restarts a running timer, too */
> > +	efm32_timer_write(TIMER_CLOCKEVENT, evt, TIMERn_TOP);
> > +	efm32_timer_write(TIMER_CLOCKEVENT, TIMERn_CMD_START, TIMERn_CMD);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct clock_event_device efm32_clock_event_device = {
> > +	.name = "efm32 clockevent (" __stringify(TIMER_CLOCKEVENT) ")",
> > +	.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_MODE_PERIODIC,
> > +	.set_mode = efm32_clock_event_set_mode,
> > +	.set_next_event = efm32_clock_event_set_next_event,
> > +	.rating = 200,
> > +};
> > +
> > +static irqreturn_t efm32_clock_event_handler(int irq, void *dev_id)
> > +{
> > +	struct clock_event_device *evt = &efm32_clock_event_device;
> > +
> > +	/* ack irq */
> > +	efm32_timer_write(TIMER_CLOCKEVENT, TIMERn_IRQ_UF, TIMERn_IFC);
> > +
> > +	evt->event_handler(evt);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static struct irqaction efm32_clock_event_irq = {
> > +	.name = "efm32 clockevent",
> > +	.flags = IRQF_TIMER,
> > +	.handler = efm32_clock_event_handler,
> > +	.dev_id = &efm32_clock_event_device,
> > +};
> > +
> > +/*
> > + * XXX: use clk_ API to get frequency and enabling of the clocks used.
> > + * Here the reset defaults are used:
> > + * - freq_{HFPERCLK} = freq_{HFCLK}
> > + *   (CMU_HFPERCLKDIV_HFPERCLKDIV = 0x0)
> > + * - freq_{HFCLK} = freq_{HFRCO}
> > + *   (CMU_CTRL_HFCLKDIV = 0x0, CMU_STATUS_HFRCOSEL = 0x1)
> > + * - freq_{HFRCO} = 14MHz
> > + *   (CMU_HFRCOCTRL_BAND = 0x3)
> > + *
> > + * So the HFPERCLK runs at 14MHz. The timer has an additional prescaler
> > + * programmed to /1024. This make the timer run at
> > + *
> > + * 	14 MHz / 1024 = 13671.875 Hz
> > + *
> > + */
> > +static void __init efm32_timer_init(void)
> > +{
> > +	/* enable CMU_HFPERCLKEN0_TIMERn for clocksource via bit-band */
> > +	__raw_writel(1, IOMEM(0x43900894 + 4 * TIMER_CLOCKSOURCE));
> > +
> > +	efm32_timer_write(TIMER_CLOCKSOURCE,
> > +			TIMERn_CTRL_PRESC_1024 |
> > +			TIMERn_CTRL_CLKSEL_PRESCHFPERCLK |
> > +			TIMERn_CTRL_MODE_UP, TIMERn_CTRL);
> > +	efm32_timer_write(TIMER_CLOCKSOURCE, TIMERn_CMD_START, TIMERn_CMD);
> > +
> > +	clocksource_mmio_init(BASEADDR_TIMER(TIMER_CLOCKSOURCE) + TIMERn_CNT,
> > +			"efm32 timer", 13672, 200, 16,
> > +			clocksource_mmio_readl_up);
> > +
> > +	/* enable CMU_HFPERCLKEN0_TIMERn for clockevent via bit-band */
> > +	__raw_writel(1, IOMEM(0x43900894 + 4 * TIMER_CLOCKEVENT));
> > +
> > +	efm32_timer_write(TIMER_CLOCKEVENT, TIMERn_IRQ_UF, TIMERn_IEN);
> > +
> > +	setup_irq(IRQ_CLOCKEVENT, &efm32_clock_event_irq);
> 
> Do you need to use setup_irq() here?  For picoxcell I used device tree 
> and so irq_of_parse_and_map() but that seemed fine for getting a timer 
> up and running.
Ok, I will evaluate that.

Thanks for your feedback,
Uwe
Ohad Ben-Cohen - Dec. 21, 2011, 5:31 p.m.
Hi Uwe,

2011/12/21 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> If you're interested in the details I can publish my tree I think.

Definitely interested - it could be very nice if you can publish that tree.

Thanks!
Ohad.
Russell King - ARM Linux - Dec. 21, 2011, 7:54 p.m.
On Wed, Dec 21, 2011 at 04:34:15PM +0000, Arnd Bergmann wrote:
> > diff --git a/arch/arm/mach-efm32/include/mach/io.h b/arch/arm/mach-efm32/include/mach/io.h
> > new file mode 100644
> > index 0000000..00b6af6
> > --- /dev/null
> > +++ b/arch/arm/mach-efm32/include/mach/io.h
> > @@ -0,0 +1,7 @@
> > +#ifndef __MACH_IO_H__
> > +#define __MACH_IO_H__
> > +
> > +#define __io(a)			__typesafe_io(a)
> > +#define __mem_pci(a)		(a)
> > +
> > +#endif /* __MACH_IO_H__ */
> 
> Can you already build with __io not defined? That should catch any drivers
> that try to access the PC-style I/O ports.

And don't forget to select NO_IOPORT in the configuration as well.

> > --- a/arch/arm/mm/Kconfig
> > +++ b/arch/arm/mm/Kconfig
> > @@ -432,7 +432,7 @@ config CPU_V7
> >  # ARMv7
> >  config CPU_V7M
> >  	bool "Support ARMv7-M processors"
> > -	depends on MACH_REALVIEW_EB && EXPERIMENTAL
> > +	depends on (MACH_REALVIEW_EB && EXPERIMENTAL) || ARCH_EFM32
> >  	select THUMB2_KERNEL
> >  	select ARM_THUMB
> >  	select CPU_32v7M
> 
> Hmm, since you *have* to enable this option for a successful build,
> wouldn't it make more sense to 'select' this from ARCH_EFM32.

It'd help if Catalin conformed to the well established way in the rest
of the CPU specific configuration.  Rather than making this depend on
realview etc, make the presentation of the prompt conditional on
realview etc.  Then ARCH_EFM32 can simply select CPU_V7M without
generating any warnings.

In other words:

config CPU_whatever
	bool "whatever" if <symbols-which-may-have-this-cpu>

and elsewhere:

config PLATFORM_which_always_has_CPU_whatever
	select CPU_whatever
Russell King - ARM Linux - Dec. 21, 2011, 7:56 p.m.
On Wed, Dec 21, 2011 at 03:48:50PM +0000, Jamie Iles wrote:
> > +#define __io(a)			__typesafe_io(a)
> 
> Do you support io ports on this platform?  If not then perhaps select 
> NO_IPORT and define __io(a) to 0?

Do not do that.  Define NO_IOPORT and *do* *not* define __io() at all to
make drivers which use PCI IO stuff fail at build time.  Don't try to
frig it into doing a NULL pointer deref.
Jamie Iles - Dec. 21, 2011, 8:09 p.m.
On Wed, Dec 21, 2011 at 07:56:02PM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 21, 2011 at 03:48:50PM +0000, Jamie Iles wrote:
> > > +#define __io(a)			__typesafe_io(a)
> > 
> > Do you support io ports on this platform?  If not then perhaps select 
> > NO_IPORT and define __io(a) to 0?
> 
> Do not do that.  Define NO_IOPORT and *do* *not* define __io() at all to
> make drivers which use PCI IO stuff fail at build time.  Don't try to
> frig it into doing a NULL pointer deref.

Fair point.  I said that because I remember that the 8250 driver uses 
ioport accessors even for !HAVE_IOPORT and that some patches to remove 
that were nacked, but I don't remember the details.  But yes, not 
defining __io would be much better!

Jamie
Nicolas Pitre - Dec. 21, 2011, 8:36 p.m.
On Wed, 21 Dec 2011, Russell King - ARM Linux wrote:

> On Wed, Dec 21, 2011 at 03:48:50PM +0000, Jamie Iles wrote:
> > > +#define __io(a)			__typesafe_io(a)
> > 
> > Do you support io ports on this platform?  If not then perhaps select 
> > NO_IPORT and define __io(a) to 0?
> 
> Do not do that.  Define NO_IOPORT and *do* *not* define __io() at all to
> make drivers which use PCI IO stuff fail at build time.  Don't try to
> frig it into doing a NULL pointer deref.

... especially since this is a !MMU platform and therefore NULL pointer 
derefs won't work as intended.


Nicolas
Uwe Kleine-König - Jan. 16, 2012, 4:29 p.m.
On Wed, Dec 21, 2011 at 07:31:56PM +0200, Ohad Ben-Cohen wrote:
> Hi Uwe,
> 
> 2011/12/21 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> > If you're interested in the details I can publish my tree I think.
> 
> Definitely interested - it could be very nice if you can publish that tree.
OK, it's online now at

	http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=refs/heads/efm32
	git://git.pengutronix.de/git/ukl/linux.git efm32

Please note that this is work in progress and has some rough edges. ...
but at least it boots on my machine.

Best regards
Uwe
Ohad Ben-Cohen - Jan. 16, 2012, 4:59 p.m.
2012/1/16 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> OK, it's online now at
>
>        http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=refs/heads/efm32
>        git://git.pengutronix.de/git/ukl/linux.git efm32
>
> Please note that this is work in progress and has some rough edges. ...
> but at least it boots on my machine.

Great, thanks !
Russell King - ARM Linux - Jan. 16, 2012, 5:40 p.m.
On Mon, Jan 16, 2012 at 05:29:33PM +0100, Uwe Kleine-König wrote:
> On Wed, Dec 21, 2011 at 07:31:56PM +0200, Ohad Ben-Cohen wrote:
> > Hi Uwe,
> > 
> > 2011/12/21 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> > > If you're interested in the details I can publish my tree I think.
> > 
> > Definitely interested - it could be very nice if you can publish that tree.
> OK, it's online now at
> 
> 	http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=refs/heads/efm32
> 	git://git.pengutronix.de/git/ukl/linux.git efm32
> 
> Please note that this is work in progress and has some rough edges. ...
> but at least it boots on my machine.

Some of your patches seem weird, like this:

        for (i = 0; i < ARRAY_SIZE(cache_policies); i++) {
-               int len = strlen(cache_policies[i].policy);
-
-               if (memcmp(p, cache_policies[i].policy, len) == 0) {
+               if (strcmp(p, cache_policies[i].policy) == 0) {

Please compare the two, specifically how many bytes of the strings in
each case are compared.  I think you'll find it's not an equivalent
change, and you need to discard the patch.

This looks buggy:
+#if defined(CONFIG_CPU_V7M)
+       .macro  setmode, mode, reg
+       .endm
+#elif defined(CONFIG_THUMB2_KERNEL)
        .macro  setmode, mode, reg
        mov     \reg, #\mode
        msr     cpsr_c, \reg

as it does nothing about the interrupt mask.

The VFP stuff - adding 'clean' which is kernel state to the _user_
_exported_ VFP hardware structure is a bad idea.  So this needlessly
causes a variation in the kernels userspace API.  Please find somewhere
else to keep kernel internal state.  (As that patch comes from Catalin,
then that comment is directed to Catalin.)

In "mtd/maps: uclinux: fix sparse warnings and coding style":

-struct map_info uclinux_ram_map = {
+static struct map_info uclinux_ram_map = {
        .name = "RAM",
-       .phys = (unsigned long)&_ebss,
-       .size = 0,
+       .phys = (resource_size_t)&_ebss,
+       .bankwidth = 4,

This doesn't match the description - it's a functional change.  Basic rule
of kernel patch generation: functional changes must _never_ _ever_, not in
a million years, be mixed with such patches.

+       *virt = (__force void *)(map->virt + from);

This says "wrong" to me loudly - by the mere fact that you're using __force.
If you're having to do that, leave the sparse warning in place.  Sparse is
_not_ a tool which must produce zero warnings for kernel code.  Sparse is
an auditing tool, and as such there are valid reasons to ignore warnings.
This is one of them.

No driver should ever use __force within its code or definitions.  That
priviledge is reserved for stuff like arch code inside IO accessors where
it really knows that it's safe.

In "ARM: new architecture for ..." I expected to find a new arch/*
directory.  What you mean is "new platform" or "new machine" not
"new architecture".  Maybe "new sub-architecture".  Good to see you're
selecting NO_IOPORT here though.

config CPU_V7M
        bool "Support ARMv7-M processors"
-       depends on MACH_REALVIEW_EB && EXPERIMENTAL
+       depends on (MACH_REALVIEW_EB && EXPERIMENTAL) || ARCH_EFM32
        select THUMB2_KERNEL
        select ARM_THUMB
        select CPU_32v7M

Ok, so EFM32 may or may not have a V7M CPU.  Cool.  What does it have
when it doesn't have a V7M CPU?  (Please fix it, like all the other CPU
stuff in this file, so that the prompt appears for those which _may_
have it, but it's selected by those which must _always_ have it.)

Your code in arch/arm/mach-efm32/include/mach/system.h won't be called
as of this merge window - that's something you should consider fixing.

In your serial driver, you really need to deal with which modes you
can't set sanely: ask Alan Cox how to do this.  POSIX has a requirement
that termios modes which can't be set should be reported back.  (eg,
if you can only do one stop bit, don't blindly ignore the request for
two stop bits.)
Catalin Marinas - Jan. 16, 2012, 6:10 p.m.
On Mon, Jan 16, 2012 at 05:40:39PM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 16, 2012 at 05:29:33PM +0100, Uwe Kleine-König wrote:
> > On Wed, Dec 21, 2011 at 07:31:56PM +0200, Ohad Ben-Cohen wrote:
> > > Hi Uwe,
> > > 
> > > 2011/12/21 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> > > > If you're interested in the details I can publish my tree I think.
> > > 
> > > Definitely interested - it could be very nice if you can publish that tree.
> > OK, it's online now at
> > 
> > 	http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=refs/heads/efm32
> > 	git://git.pengutronix.de/git/ukl/linux.git efm32
> > 
> > Please note that this is work in progress and has some rough edges. ...
> > but at least it boots on my machine.
...
> This looks buggy:
> +#if defined(CONFIG_CPU_V7M)
> +       .macro  setmode, mode, reg
> +       .endm
> +#elif defined(CONFIG_THUMB2_KERNEL)
>         .macro  setmode, mode, reg
>         mov     \reg, #\mode
>         msr     cpsr_c, \reg
> 
> as it does nothing about the interrupt mask.

The interrupt mask is not part of CPSR on the M profile, so setmode
didn't make much sense. Looking through the kernel, there are only two
calls to setmode that would disable interrupts, so we need to make sure
they are indeed disabled (though given how the M profile works, with
priorities, we could already be safe; it needs checking though).

> The VFP stuff - adding 'clean' which is kernel state to the _user_
> _exported_ VFP hardware structure is a bad idea.  So this needlessly
> causes a variation in the kernels userspace API.  Please find somewhere
> else to keep kernel internal state.  (As that patch comes from Catalin,
> then that comment is directed to Catalin.)

Are you sure we export vfp_hard_struct to user? That's a kernel-only
structure (and it's not by any means stable, given the number of
#ifdef's it has). I would also argue that 'clean' is a hardware state
(inferred from the exception return value).
Russell King - ARM Linux - Jan. 16, 2012, 7:06 p.m.
On Mon, Jan 16, 2012 at 06:10:02PM +0000, Catalin Marinas wrote:
> On Mon, Jan 16, 2012 at 05:40:39PM +0000, Russell King - ARM Linux wrote:
> > The VFP stuff - adding 'clean' which is kernel state to the _user_
> > _exported_ VFP hardware structure is a bad idea.  So this needlessly
> > causes a variation in the kernels userspace API.  Please find somewhere
> > else to keep kernel internal state.  (As that patch comes from Catalin,
> > then that comment is directed to Catalin.)
> 
> Are you sure we export vfp_hard_struct to user? That's a kernel-only
> structure (and it's not by any means stable, given the number of
> #ifdef's it has). I would also argue that 'clean' is a hardware state
> (inferred from the exception return value).

Actually, looking at arch/arm/kernel/ptrace.c, we only export the
fpregs and fpscr, so this should be fine.

Still, I don't see why we need this 'clean' state, when normal VFP
doesn't need it.  Maybe you could explain why it's necessary?
Uwe Kleine-König - Jan. 17, 2012, 10:17 a.m.
Hello Russell,

I'm glad you're replying though my tree was just intended to show a
prove of concept, not a tree ready to be merged.

On Mon, Jan 16, 2012 at 05:40:39PM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 16, 2012 at 05:29:33PM +0100, Uwe Kleine-König wrote:
> > On Wed, Dec 21, 2011 at 07:31:56PM +0200, Ohad Ben-Cohen wrote:
> > > Hi Uwe,
> > > 
> > > 2011/12/21 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> > > > If you're interested in the details I can publish my tree I think.
> > > 
> > > Definitely interested - it could be very nice if you can publish that tree.
> > OK, it's online now at
> > 
> > 	http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=refs/heads/efm32
> > 	git://git.pengutronix.de/git/ukl/linux.git efm32
> > 
> > Please note that this is work in progress and has some rough edges. ...
> > but at least it boots on my machine.
> 
> Some of your patches seem weird, like this:
> 
>         for (i = 0; i < ARRAY_SIZE(cache_policies); i++) {
> -               int len = strlen(cache_policies[i].policy);
> -
> -               if (memcmp(p, cache_policies[i].policy, len) == 0) {
> +               if (strcmp(p, cache_policies[i].policy) == 0) {
> 
> Please compare the two, specifically how many bytes of the strings in
> each case are compared.  I think you'll find it's not an equivalent
> change, and you need to discard the patch.
OK, so you say it is intended that I can do

	early_cachepolicy("writebackyourmother");

with the same result as

	early_cachepolicy("writeback");

? Maybe a comment would be great to have in place then about
why this is done. I couldn't find a user apart from

	early_param("cachepolicy", early_cachepolicy);

and this parameter isn't described in
Documentation/kernel-parameters.txt.

I would understand if only strlen(p) bytes were checked which would
allow to shorten the parameter. (But then

	cachepolicy=

would have a meaning (though I didn't test -- I think it would select
uncached) which is strange.) All in all I put the patch there as a
reminder for me to ask if everything the function does is intended.

> This looks buggy:
> +#if defined(CONFIG_CPU_V7M)
> +       .macro  setmode, mode, reg
> +       .endm
> +#elif defined(CONFIG_THUMB2_KERNEL)
>         .macro  setmode, mode, reg
>         mov     \reg, #\mode
>         msr     cpsr_c, \reg
> 
> as it does nothing about the interrupt mask.
right, good catch.

> The VFP stuff - adding 'clean' which is kernel state to the _user_
> _exported_ VFP hardware structure is a bad idea.  So this needlessly
> causes a variation in the kernels userspace API.  Please find somewhere
> else to keep kernel internal state.  (As that patch comes from Catalin,
> then that comment is directed to Catalin.)
> 
> In "mtd/maps: uclinux: fix sparse warnings and coding style":
> 
> -struct map_info uclinux_ram_map = {
> +static struct map_info uclinux_ram_map = {
>         .name = "RAM",
> -       .phys = (unsigned long)&_ebss,
> -       .size = 0,
> +       .phys = (resource_size_t)&_ebss,
> +       .bankwidth = 4,
> 
> This doesn't match the description - it's a functional change.  Basic rule
> of kernel patch generation: functional changes must _never_ _ever_, not in
I stumbled about that, too, but a deeper look shows that

	.bankwidth = 4,

was introduced in place of the removed

	mapp->bankwidth = 4;

in uclinux_mtd_init(). I didn't check the details because it's not my
patch and I don't intend (yet) to mainline it myself. (Marc: hint hint!)

> a million years, be mixed with such patches.
> 
> +       *virt = (__force void *)(map->virt + from);
> 
> This says "wrong" to me loudly - by the mere fact that you're using __force.
> If you're having to do that, leave the sparse warning in place.  Sparse is
> _not_ a tool which must produce zero warnings for kernel code.  Sparse is
> an auditing tool, and as such there are valid reasons to ignore warnings.
> This is one of them.
> 
> No driver should ever use __force within its code or definitions.  That
> priviledge is reserved for stuff like arch code inside IO accessors where
> it really knows that it's safe.
> 
> In "ARM: new architecture for ..." I expected to find a new arch/*
> directory.  What you mean is "new platform" or "new machine" not
> "new architecture".  Maybe "new sub-architecture".  Good to see you're
> selecting NO_IOPORT here though.
Yeah, I learned NO_IOPORT from the first post of the "new architecture"
patch :-)

> config CPU_V7M
>         bool "Support ARMv7-M processors"
> -       depends on MACH_REALVIEW_EB && EXPERIMENTAL
> +       depends on (MACH_REALVIEW_EB && EXPERIMENTAL) || ARCH_EFM32
>         select THUMB2_KERNEL
>         select ARM_THUMB
>         select CPU_32v7M
> 
> Ok, so EFM32 may or may not have a V7M CPU.  Cool.  What does it have
> when it doesn't have a V7M CPU?  (Please fix it, like all the other CPU
> stuff in this file, so that the prompt appears for those which _may_
> have it, but it's selected by those which must _always_ have it.)
Yeah, this was pointed out in the review of my first post, too. Didn't
come around yet to fix that.
 
> Your code in arch/arm/mach-efm32/include/mach/system.h won't be called
> as of this merge window - that's something you should consider fixing.
yeah, I'm aware of this change and intend to update the base when
3.3-rc1 is out.
 
> In your serial driver, you really need to deal with which modes you
> can't set sanely: ask Alan Cox how to do this.  POSIX has a requirement
> that termios modes which can't be set should be reported back.  (eg,
> if you can only do one stop bit, don't blindly ignore the request for
> two stop bits.)
Ah, I thought CSTOPB means "use 1 stop bit" but it seems to request 2.
Will fix and send out a v3.

Thanks for your time
Uwe
Catalin Marinas - Jan. 17, 2012, 11:32 a.m.
On Mon, Jan 16, 2012 at 07:06:37PM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 16, 2012 at 06:10:02PM +0000, Catalin Marinas wrote:
> > On Mon, Jan 16, 2012 at 05:40:39PM +0000, Russell King - ARM Linux wrote:
> > > The VFP stuff - adding 'clean' which is kernel state to the _user_
> > > _exported_ VFP hardware structure is a bad idea.  So this needlessly
> > > causes a variation in the kernels userspace API.  Please find somewhere
> > > else to keep kernel internal state.  (As that patch comes from Catalin,
> > > then that comment is directed to Catalin.)
> > 
> > Are you sure we export vfp_hard_struct to user? That's a kernel-only
> > structure (and it's not by any means stable, given the number of
> > #ifdef's it has). I would also argue that 'clean' is a hardware state
> > (inferred from the exception return value).
> 
> Actually, looking at arch/arm/kernel/ptrace.c, we only export the
> fpregs and fpscr, so this should be fine.
> 
> Still, I don't see why we need this 'clean' state, when normal VFP
> doesn't need it.  Maybe you could explain why it's necessary?

The FP support on the M profile is a bit different (in terms of control
registers) from the A/R profiles. The M profile has built-in knowledge
of the AAPCS for automatically preservation of certain registers during
exceptions and it can also do lazy saving of the S0-S15 FP registers.
In general the M processors are meant to be simpler to use without a
complex OS (especially if thread switching is done synchronously at SVC
time rather than during interrupts).

For example, when user space gets an exception, the M core saves R0-R3,
R12, LR, PC, xPSR on the user stack automatically. It also preserves
space for S0-S15 and FPSCR but does not save them (it remembers the
address though for lazy saving if new thread uses the FP). If another
thread tries to use the FP for the first time, it saves the old FP state
and initialises a new one for the current thread automatically. But once
a thread touched the FP, its state is no longer 'clean' and it is
automatically loaded (non-lazily) from the stack when switching to such
thread while saving the old one on the previous stack. When switching
between two threads that have never used the FP, the processor does not
do any FP state switching.

However, it does not save S16-S31 as the AAPCS specifies that they are
caller-saved. Linux needs to do this and it uses the 'clean' state to
detect whether a thread used the FP or not. The disadvantage is that if
a thread ever used FP, its state is always saved/restored at context
switch.

Of course, we can change all this and disable the hardware automatic
saving while implementing a pure software lazy switching solution using
the CPACR (there is no FPEXC). But at the time this was the simplest
implementation, given that Cortex-M3 doesn't even have an FP unit (it
came with Cortex-M4) and most M3 software around didn't touch the FP at
all.

BTW, if you are fine with trying to get the M support into mainline, I'm
happy to revisit the code (probably with Uwe's help given that I don't
have much time available).

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 8555445..68de356 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -400,6 +400,15 @@  config ARCH_EBSA110
 	  Ethernet interface, two PCMCIA sockets, two serial ports and a
 	  parallel port.
 
+config ARCH_EFM32
+	bool "EnergyMicro Cortex M3 Platform"
+	depends on !MMU
+	select CPU_V7M
+	select ARM_NVIC
+	select GENERIC_CLOCKEVENTS
+	select NO_DMA
+	select CLKSRC_MMIO
+
 config ARCH_EP93XX
 	bool "EP93xx-based"
 	select CPU_ARM920T
@@ -1699,7 +1708,7 @@  source "mm/Kconfig"
 config FORCE_MAX_ZONEORDER
 	int "Maximum zone order" if ARCH_SHMOBILE
 	range 11 64 if ARCH_SHMOBILE
-	default "9" if SA1111
+	default "9" if SA1111 || ARCH_EFM32
 	default "11"
 	help
 	  The kernel memory allocator divides physically contiguous memory
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index c5213e7..cdec99b 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -128,6 +128,14 @@  choice
 		  Say Y here if you want the debug print routines to direct
 		  their output to the second serial port on these devices.
 
+	config DEBUG_EFM32_USART1
+		bool "Kernel low-level debugging messages via USART1"
+		depends on ARCH_EFM32
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to the second serial port on efm32 based
+		  machines.
+
 	config DEBUG_HIGHBANK_UART
 		bool "Kernel low-level debugging messages via Highbank UART"
 		depends on ARCH_HIGHBANK
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 042fdec..b955c46 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -142,6 +142,7 @@  machine-$(CONFIG_ARCH_CNS3XXX)		:= cns3xxx
 machine-$(CONFIG_ARCH_DAVINCI)		:= davinci
 machine-$(CONFIG_ARCH_DOVE)		:= dove
 machine-$(CONFIG_ARCH_EBSA110)		:= ebsa110
+machine-$(CONFIG_ARCH_EFM32)		:= efm32
 machine-$(CONFIG_ARCH_EP93XX)		:= ep93xx
 machine-$(CONFIG_ARCH_GEMINI)		:= gemini
 machine-$(CONFIG_ARCH_H720X)		:= h720x
diff --git a/arch/arm/boot/dts/efm32gg-dk3750.dts b/arch/arm/boot/dts/efm32gg-dk3750.dts
new file mode 100644
index 0000000..a4aa4f3
--- /dev/null
+++ b/arch/arm/boot/dts/efm32gg-dk3750.dts
@@ -0,0 +1,41 @@ 
+/dts-v1/;
+/include/ "skeleton.dtsi"
+
+/ {
+	model = "Energy Micro Giant Gecko Development Kit";
+	compatible = "efm32,dk3750";
+
+	aliases {
+		serial1 = &uart1;
+	};
+
+	nvic: nv-interrupt-controller@0xe0000000  {
+		compatible = "arm,cortex-m3-nvic";
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		reg = <0xe0000000 0x4000>;
+	};
+
+	chosen {
+		bootargs = "console=ttyefm1,115200 init=/linuxrc ignore_loglevel ihash_entries=64 dhash_entries=64";
+	};
+
+	memory {
+		reg = <0x80000000 0x100000>;
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		interrupt-parent = <&nvic>;
+		ranges;
+
+		uart1: uart@0x4000c400 { /* USART1 */
+			compatible = "efm32,usart";
+			reg = <0x4000c400 0x400>;
+			interrupts = <15>;
+			status = "ok";
+		};
+	};
+};
diff --git a/arch/arm/mach-efm32/Makefile b/arch/arm/mach-efm32/Makefile
new file mode 100644
index 0000000..10a3426
--- /dev/null
+++ b/arch/arm/mach-efm32/Makefile
@@ -0,0 +1,3 @@ 
+obj-y += clk.o time.o
+
+obj-$(CONFIG_OF) += dtmachine.o
diff --git a/arch/arm/mach-efm32/Makefile.boot b/arch/arm/mach-efm32/Makefile.boot
new file mode 100644
index 0000000..385e93a
--- /dev/null
+++ b/arch/arm/mach-efm32/Makefile.boot
@@ -0,0 +1 @@ 
+dtb-$(CONFIG_MACH_EFM32GG_DK3750) += efm32gg-dk3750.dtb
diff --git a/arch/arm/mach-efm32/clk.c b/arch/arm/mach-efm32/clk.c
new file mode 100644
index 0000000..86fadc8
--- /dev/null
+++ b/arch/arm/mach-efm32/clk.c
@@ -0,0 +1,30 @@ 
+#include <linux/clk.h>
+#include <linux/export.h>
+
+struct clk *clk_get(struct device *dev, const char *id)
+{
+	return NULL;
+}
+EXPORT_SYMBOL(clk_get);
+
+int clk_enable(struct clk *clk)
+{
+	return 0;
+}
+EXPORT_SYMBOL(clk_enable);
+
+void clk_disable(struct clk *clk)
+{
+}
+EXPORT_SYMBOL(clk_disable);
+
+unsigned long clk_get_rate(struct clk *clk)
+{
+	return 14000000;
+}
+EXPORT_SYMBOL(clk_get_rate);
+
+void clk_put(struct clk *clk)
+{
+}
+EXPORT_SYMBOL(clk_put);
diff --git a/arch/arm/mach-efm32/common.h b/arch/arm/mach-efm32/common.h
new file mode 100644
index 0000000..f545224
--- /dev/null
+++ b/arch/arm/mach-efm32/common.h
@@ -0,0 +1,7 @@ 
+#ifdef __ASSEMBLER__
+#define IOMEM(addr)     (addr)
+#else
+#define IOMEM(addr)     ((void __force __iomem *)(addr))
+#endif
+
+extern struct sys_timer efm32_timer;
diff --git a/arch/arm/mach-efm32/dtmachine.c b/arch/arm/mach-efm32/dtmachine.c
new file mode 100644
index 0000000..42d091c
--- /dev/null
+++ b/arch/arm/mach-efm32/dtmachine.c
@@ -0,0 +1,47 @@ 
+#include <linux/kernel.h>
+#include <linux/pinctrl/machine.h>
+#include <linux/irqdomain.h>
+#include <linux/of_platform.h>
+#include <linux/of_irq.h>
+
+#include <asm/hardware/nvic.h>
+#include <asm/mach/arch.h>
+#include <asm/mach/time.h>
+
+#include "common.h"
+
+static void __init efm32_nvic_add_irq_domain(struct device_node *np,
+		struct device_node *interrupt_parent)
+{
+	irq_domain_add_simple(np, 0);
+}
+
+static const struct of_device_id efm32_irq_match[] __initconst = {
+	{
+		.compatible = "arm,cortex-m3-nvic",
+		.data = efm32_nvic_add_irq_domain,
+	}, {
+		/* sentinel */
+	}
+};
+
+static void __init efm32_init(void)
+{
+	int ret;
+
+	of_irq_init(efm32_irq_match);
+
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+}
+
+static const char *const efm32gg_compat[] __initconst = {
+	"efm32,dk3750",
+	NULL
+};
+
+DT_MACHINE_START(EFM32DT, "EFM32 (Device Tree Support)")
+	.init_irq = nvic_init,
+	.timer = &efm32_timer,
+	.init_machine = efm32_init,
+	.dt_compat = efm32gg_compat,
+MACHINE_END
diff --git a/arch/arm/mach-efm32/include/mach/debug-macro.S b/arch/arm/mach-efm32/include/mach/debug-macro.S
new file mode 100644
index 0000000..cbf37f3
--- /dev/null
+++ b/arch/arm/mach-efm32/include/mach/debug-macro.S
@@ -0,0 +1,32 @@ 
+/*
+ * 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.
+ */
+
+		.macro	addruart,rx,tmp
+		mov	\rx,      #0x40000000
+		orr	\rx, \rx, #0x0000C400		/* USART1 */
+		.endm
+
+#define	USARTn_DR	0x34
+#define	USARTn_FR	0x10
+#define	USARTn_FR_TXFF	0x0020
+
+		.macro	senduart,rd,rx
+		strb	\rd, [\rx, #USARTn_DR]
+		.endm
+
+		.macro	waituart,rd,rx
+1001:		ldr	\rd, [\rx, #USARTn_FR]
+		tst	\rd, #USARTn_FR_TXFF
+		it eq
+		beq	1001b
+		.endm
+
+		.macro	busyuart,rd,rx
+1001:		ldr	\rd, [\rx, USARTn_FR]
+		tst	\rd, USARTn_FR_TXFF
+		it ne
+		bne	1001b
+		.endm
diff --git a/arch/arm/mach-efm32/include/mach/entry-macro.S b/arch/arm/mach-efm32/include/mach/entry-macro.S
new file mode 100644
index 0000000..75b304a
--- /dev/null
+++ b/arch/arm/mach-efm32/include/mach/entry-macro.S
@@ -0,0 +1,12 @@ 
+/*
+ *
+ */
+#include <asm/hardware/gic.h>
+
+		.macro	get_irqnr_preamble, base, tmp
+		ldr	\base, =gic_cpu_base_addr
+		ldr	\base, [\base]
+		.endm
+
+		.macro	arch_ret_to_user, tmp1, tmp2
+		.endm
diff --git a/arch/arm/mach-efm32/include/mach/io.h b/arch/arm/mach-efm32/include/mach/io.h
new file mode 100644
index 0000000..00b6af6
--- /dev/null
+++ b/arch/arm/mach-efm32/include/mach/io.h
@@ -0,0 +1,7 @@ 
+#ifndef __MACH_IO_H__
+#define __MACH_IO_H__
+
+#define __io(a)			__typesafe_io(a)
+#define __mem_pci(a)		(a)
+
+#endif /* __MACH_IO_H__ */
diff --git a/arch/arm/mach-efm32/include/mach/irqs.h b/arch/arm/mach-efm32/include/mach/irqs.h
new file mode 100644
index 0000000..5fa84db
--- /dev/null
+++ b/arch/arm/mach-efm32/include/mach/irqs.h
@@ -0,0 +1,6 @@ 
+#ifndef __MACH_IRQS_H__
+#define __MACH_IRQS_H__
+
+#define NR_IRQS			37
+
+#endif /* __MACH_IRQS_H__ */
diff --git a/arch/arm/mach-efm32/include/mach/system.h b/arch/arm/mach-efm32/include/mach/system.h
new file mode 100644
index 0000000..619222c
--- /dev/null
+++ b/arch/arm/mach-efm32/include/mach/system.h
@@ -0,0 +1,18 @@ 
+#ifndef __MACH_SYSTEM_H__
+#define __MACH_SYSTEM_H__
+
+#include <asm/io.h>
+
+static inline void arch_idle(void)
+{
+	cpu_do_idle();
+}
+
+static inline void arch_reset(char mode, const char *cmd)
+{
+	/* XXX: move this to (say) cpuv7m_reset */
+	dsb();
+	__raw_writel(0x05fa0004, (void __iomem *)0xe000ed0c);
+	dsb();
+}
+#endif /* __MACH_SYSTEM_H__ */
diff --git a/arch/arm/mach-efm32/include/mach/timex.h b/arch/arm/mach-efm32/include/mach/timex.h
new file mode 100644
index 0000000..b408dce
--- /dev/null
+++ b/arch/arm/mach-efm32/include/mach/timex.h
@@ -0,0 +1,7 @@ 
+#ifndef __MACH_TIMEX_H__
+#define __MACH_TIMEX_H__
+
+/* just a bogus value */
+#define CLOCK_TICK_RATE	12345678
+
+#endif /* __MACH_TIMEX_H__ */
diff --git a/arch/arm/mach-efm32/time.c b/arch/arm/mach-efm32/time.c
new file mode 100644
index 0000000..65f93a3
--- /dev/null
+++ b/arch/arm/mach-efm32/time.c
@@ -0,0 +1,166 @@ 
+#include <linux/kernel.h>
+#include <linux/clocksource.h>
+#include <linux/clockchips.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/stringify.h>
+
+#include <asm/mach/time.h>
+
+#include <common.h>
+
+#define BASEADDR_TIMER(n)		IOMEM(0x40010000 + (n) * 0x400)
+
+#define TIMERn_CTRL			0x00
+#define TIMERn_CTRL_PRESC(val)			(((val) & 0xf) << 24)
+#define TIMERn_CTRL_PRESC_1024			TIMERn_CTRL_PRESC(10)
+#define TIMERn_CTRL_CLKSEL(val)			(((val) & 0x3) << 16)
+#define TIMERn_CTRL_CLKSEL_PRESCHFPERCLK	TIMERn_CTRL_CLKSEL(0)
+#define TIMERn_CTRL_OSMEN			0x00000010
+#define TIMERn_CTRL_MODE(val)			(((val) & 0x3) <<  0)
+#define TIMERn_CTRL_MODE_UP			TIMERn_CTRL_MODE(0)
+#define TIMERn_CTRL_MODE_DOWN			TIMERn_CTRL_MODE(1)
+
+#define TIMERn_CMD			0x04
+#define TIMERn_CMD_START			0x1
+#define TIMERn_CMD_STOP				0x2
+
+#define TIMERn_IEN			0x0c
+#define TIMERn_IF			0x10
+#define TIMERn_IFS			0x14
+#define TIMERn_IFC			0x18
+#define TIMERn_IRQ_UF				0x2
+#define TIMERn_IRQ_OF				0x1
+
+#define TIMERn_TOP			0x1c
+#define TIMERn_CNT			0x24
+
+#define TIMER_CLOCKSOURCE	1
+#define TIMER_CLOCKEVENT	2
+#define IRQ_CLOCKEVENT		13
+
+static void efm32_timer_write(unsigned timerno, u32 val, unsigned offset)
+{
+	__raw_writel(val, BASEADDR_TIMER(timerno) + offset);
+}
+
+static void efm32_clock_event_set_mode(enum clock_event_mode mode,
+		struct clock_event_device *unused)
+{
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		efm32_timer_write(TIMER_CLOCKEVENT,
+				TIMERn_CMD_STOP, TIMERn_CMD);
+		efm32_timer_write(TIMER_CLOCKEVENT, 137, TIMERn_TOP);
+		efm32_timer_write(TIMER_CLOCKEVENT,
+				TIMERn_CTRL_PRESC_1024 |
+				TIMERn_CTRL_CLKSEL_PRESCHFPERCLK |
+				TIMERn_CTRL_MODE_DOWN, TIMERn_CTRL);
+		efm32_timer_write(TIMER_CLOCKEVENT,
+				TIMERn_CMD_START, TIMERn_CMD);
+		break;
+
+	case CLOCK_EVT_MODE_ONESHOT:
+		efm32_timer_write(TIMER_CLOCKEVENT,
+				TIMERn_CMD_STOP, TIMERn_CMD);
+		efm32_timer_write(TIMER_CLOCKEVENT,
+				TIMERn_CTRL_PRESC_1024 |
+				TIMERn_CTRL_CLKSEL_PRESCHFPERCLK |
+				TIMERn_CTRL_OSMEN |
+				TIMERn_CTRL_MODE_DOWN, TIMERn_CTRL);
+		break;
+
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+		efm32_timer_write(TIMER_CLOCKEVENT, TIMERn_CMD_STOP,
+				TIMERn_CMD);
+		break;
+
+	case CLOCK_EVT_MODE_RESUME:
+		break;
+	}
+}
+
+static int efm32_clock_event_set_next_event(unsigned long evt,
+		struct clock_event_device *unused)
+{
+	/* writing START to CMD restarts a running timer, too */
+	efm32_timer_write(TIMER_CLOCKEVENT, evt, TIMERn_TOP);
+	efm32_timer_write(TIMER_CLOCKEVENT, TIMERn_CMD_START, TIMERn_CMD);
+
+	return 0;
+}
+
+static struct clock_event_device efm32_clock_event_device = {
+	.name = "efm32 clockevent (" __stringify(TIMER_CLOCKEVENT) ")",
+	.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_MODE_PERIODIC,
+	.set_mode = efm32_clock_event_set_mode,
+	.set_next_event = efm32_clock_event_set_next_event,
+	.rating = 200,
+};
+
+static irqreturn_t efm32_clock_event_handler(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = &efm32_clock_event_device;
+
+	/* ack irq */
+	efm32_timer_write(TIMER_CLOCKEVENT, TIMERn_IRQ_UF, TIMERn_IFC);
+
+	evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+static struct irqaction efm32_clock_event_irq = {
+	.name = "efm32 clockevent",
+	.flags = IRQF_TIMER,
+	.handler = efm32_clock_event_handler,
+	.dev_id = &efm32_clock_event_device,
+};
+
+/*
+ * XXX: use clk_ API to get frequency and enabling of the clocks used.
+ * Here the reset defaults are used:
+ * - freq_{HFPERCLK} = freq_{HFCLK}
+ *   (CMU_HFPERCLKDIV_HFPERCLKDIV = 0x0)
+ * - freq_{HFCLK} = freq_{HFRCO}
+ *   (CMU_CTRL_HFCLKDIV = 0x0, CMU_STATUS_HFRCOSEL = 0x1)
+ * - freq_{HFRCO} = 14MHz
+ *   (CMU_HFRCOCTRL_BAND = 0x3)
+ *
+ * So the HFPERCLK runs at 14MHz. The timer has an additional prescaler
+ * programmed to /1024. This make the timer run at
+ *
+ * 	14 MHz / 1024 = 13671.875 Hz
+ *
+ */
+static void __init efm32_timer_init(void)
+{
+	/* enable CMU_HFPERCLKEN0_TIMERn for clocksource via bit-band */
+	__raw_writel(1, IOMEM(0x43900894 + 4 * TIMER_CLOCKSOURCE));
+
+	efm32_timer_write(TIMER_CLOCKSOURCE,
+			TIMERn_CTRL_PRESC_1024 |
+			TIMERn_CTRL_CLKSEL_PRESCHFPERCLK |
+			TIMERn_CTRL_MODE_UP, TIMERn_CTRL);
+	efm32_timer_write(TIMER_CLOCKSOURCE, TIMERn_CMD_START, TIMERn_CMD);
+
+	clocksource_mmio_init(BASEADDR_TIMER(TIMER_CLOCKSOURCE) + TIMERn_CNT,
+			"efm32 timer", 13672, 200, 16,
+			clocksource_mmio_readl_up);
+
+	/* enable CMU_HFPERCLKEN0_TIMERn for clockevent via bit-band */
+	__raw_writel(1, IOMEM(0x43900894 + 4 * TIMER_CLOCKEVENT));
+
+	efm32_timer_write(TIMER_CLOCKEVENT, TIMERn_IRQ_UF, TIMERn_IEN);
+
+	setup_irq(IRQ_CLOCKEVENT, &efm32_clock_event_irq);
+
+	/* XXX: tune min_delta */
+	clockevents_config_and_register(&efm32_clock_event_device,
+			13672, 0xf, 0xffff);
+}
+
+struct sys_timer efm32_timer = {
+	.init = efm32_timer_init,
+};
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index 2bc1a68..5b81e46 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -432,7 +432,7 @@  config CPU_V7
 # ARMv7
 config CPU_V7M
 	bool "Support ARMv7-M processors"
-	depends on MACH_REALVIEW_EB && EXPERIMENTAL
+	depends on (MACH_REALVIEW_EB && EXPERIMENTAL) || ARCH_EFM32
 	select THUMB2_KERNEL
 	select ARM_THUMB
 	select CPU_32v7M