ARM: new architecture for Energy Micro's EFM32 Cortex-M3 SoCs

arch/arm/boot/dts/efm32gg-dk3750.dts new file +7 -0

new file mode 100644
index 0000000..a4aa4f3
@@ -0,0 +1,41 @@
+/dts-v1/;
+/include/ "skeleton.dtsi"
+
+/ {
+	model = "Energy Micro Giant Gecko Development Kit";
+	compatible = "efm32,dk3750";
+

arch/arm/boot/dts/efm32gg-dk3750.dts new file +18 -0

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

arch/arm/mach-efm32/common.h new file +0 -0

new file mode 100644
index 0000000..f545224

arch/arm/mach-efm32/common.h new file +1 -0

+extern struct sys_timer efm32_timer;

arch/arm/mach-efm32/dtmachine.c new file +7 -0

new file mode 100644
index 0000000..42d091c
@@ -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>

arch/arm/mach-efm32/dtmachine.c new file +4 -0

+	.timer = &efm32_timer,
+	.init_machine = efm32_init,
+	.dt_compat = efm32gg_compat,
+MACHINE_END

arch/arm/mach-efm32/include/mach/io.h new file +0 -0

new file mode 100644
index 0000000..00b6af6

arch/arm/mach-efm32/include/mach/irqs.h new file +2 -0

+
+#endif /* __MACH_IRQS_H__ */

arch/arm/mach-efm32/include/mach/system.h new file +8 -0

new file mode 100644
index 0000000..619222c
@@ -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();

arch/arm/mach-efm32/time.c new file +0 -0

new file mode 100644
index 0000000..65f93a3

arch/arm/mach-efm32/time.c new file +22 -0

+
+#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
+

arch/arm/mach-efm32/time.c new file +103 -0

+}
+
+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);
+

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

[...]

Quoted 19 lines

arch/arm/boot/dts/efm32gg-dk3750.dts new file +13 -0

new file mode 100644
index 0000000..a4aa4f3
@@ -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 at 0xe0000000  {
+		compatible = "arm,cortex-m3-nvic";

arch/arm/boot/dts/efm32gg-dk3750.dts line 17

Quoted 4 lines

arch/arm/boot/dts/efm32gg-dk3750.dts new file +4 -0

+		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...

arch/arm/boot/dts/efm32gg-dk3750.dts line 23

Quoted 5 lines

arch/arm/boot/dts/efm32gg-dk3750.dts new file +5 -0

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

I think "memory at 80000000" is preferred here.

Quoted 24 lines

arch/arm/boot/dts/efm32gg-dk3750.dts new file +18 -0

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

arch/arm/mach-efm32/common.h new file +0 -0

new file mode 100644
index 0000000..f545224

arch/arm/mach-efm32/common.h line 5

Quoted 6 lines

arch/arm/mach-efm32/common.h new file +5 -0

@@ -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?

Quoted 20 lines

arch/arm/mach-efm32/common.h new file +1 -0

+extern struct sys_timer efm32_timer;

arch/arm/mach-efm32/dtmachine.c new file +13 -0

new file mode 100644
index 0000000..42d091c
@@ -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,

arch/arm/mach-efm32/dtmachine.c line 17

Quoted 4 lines

arch/arm/mach-efm32/dtmachine.c new file +4 -0

+		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?

Quoted 7 lines

arch/arm/mach-efm32/dtmachine.c new file +7 -0

+
+static const struct of_device_id efm32_irq_match[] __initconst = {
+	{
+		.compatible = "arm,cortex-m3-nvic",
+		.data = efm32_nvic_add_irq_domain,
+	}, {
+		/* sentinel */

arch/arm/mach-efm32/dtmachine.c line 30

Quoted 6 lines

arch/arm/mach-efm32/dtmachine.c new file +6 -0

+	}
+};
+
+static void __init efm32_init(void)
+{
+	int ret;

This doesn't appear to be used.

arch/arm/mach-efm32/dtmachine.c line 32

Quoted 2 lines

arch/arm/mach-efm32/dtmachine.c new file +2 -0

+
+	of_irq_init(efm32_irq_match);

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

Quoted 5 lines

arch/arm/mach-efm32/dtmachine.c new file +5 -0

+
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+}
+
+static const char *const efm32gg_compat[] __initconst = {

arch/arm/mach-efm32/dtmachine.c line 43

Quoted 6 lines

arch/arm/mach-efm32/dtmachine.c new file +6 -0

+	"efm32,dk3750",
+	NULL
+};
+
+DT_MACHINE_START(EFM32DT, "EFM32 (Device Tree Support)")
+	.init_irq = nvic_init,

This doesn't exist...

Quoted 11 lines

arch/arm/mach-efm32/dtmachine.c new file +4 -0

+	.timer = &efm32_timer,
+	.init_machine = efm32_init,
+	.dt_compat = efm32gg_compat,
+MACHINE_END

arch/arm/mach-efm32/include/mach/io.h new file +0 -0

new file mode 100644
index 0000000..00b6af6

arch/arm/mach-efm32/include/mach/io.h line 4

Quoted 4 lines

arch/arm/mach-efm32/include/mach/io.h new file +4 -0

@@ -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?

arch/arm/mach-efm32/include/mach/irqs.h line 4

Quoted 13 lines

arch/arm/mach-efm32/include/mach/io.h new file +3 -0

+#define __mem_pci(a)		(a)
+
+#endif /* __MACH_IO_H__ */

arch/arm/mach-efm32/include/mach/irqs.h new file +4 -0

new file mode 100644
index 0000000..5fa84db
@@ -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...

Quoted 22 lines

arch/arm/mach-efm32/include/mach/irqs.h new file +2 -0

+
+#endif /* __MACH_IRQS_H__ */

arch/arm/mach-efm32/include/mach/system.h new file +14 -0

new file mode 100644
index 0000000..619222c
@@ -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();

arch/arm/mach-efm32/include/mach/system.h line 18

Quoted 4 lines

arch/arm/mach-efm32/include/mach/system.h new file +4 -0

+	__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.

Quoted 6 lines

arch/arm/mach-efm32/time.c new file +0 -0

new file mode 100644
index 0000000..65f93a3

arch/arm/mach-efm32/time.c line 10

Quoted 10 lines

arch/arm/mach-efm32/time.c new file +10 -0

@@ -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?

arch/arm/mach-efm32/time.c line 12

Quoted 3 lines

arch/arm/mach-efm32/time.c new file +2 -0

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

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

Quoted 22 lines

arch/arm/mach-efm32/time.c new file +22 -0

+
+#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
+

arch/arm/mach-efm32/time.c line 44

Quoted 10 lines

arch/arm/mach-efm32/time.c new file +10 -0

+#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().

Quoted 109 lines

arch/arm/mach-efm32/time.c new file +109 -0

+}
+
+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));

arch/arm/mach-efm32/time.c line 157

Quoted 4 lines

arch/arm/mach-efm32/time.c new file +4 -0

+
+	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.

Quoted 1 line

+#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?

Quoted 4 lines · Jamie Iles

Quoted 1 line

+#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.

Quoted 4 lines · Jamie Iles

Quoted 1 line

+#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.

Signed-off-by Uwe Kleine-K?nig <[email protected]>

---
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.

arch/arm/boot/dts/efm32gg-dk3750.dts new file +4 -0

+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";

Quoted 8 lines

arch/arm/mach-efm32/include/mach/io.h new file +2 -0

new file mode 100644
index 0000000..00b6af6
@@ -0,0 +1,7 @@
+#ifndef __MACH_IO_H__
+#define __MACH_IO_H__

arch/arm/mach-efm32/include/mach/io.h line 7

Quoted 5 lines

arch/arm/mach-efm32/include/mach/io.h new file +5 -0

+
+#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.

Quoted 6 lines

arch/arm/mm/Kconfig +0 -0

@@ -432,7 +432,7 @@ config CPU_V7
 # ARMv7
 config CPU_V7M
 	bool "Support ARMv7-M processors"

arch/arm/mm/Kconfig config CPU_V7 · line 438

Quoted 5 lines

arch/arm/mm/Kconfig +1 -1

-	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.

If you're interested in the details I can publish my tree I think.

Hi Uwe,

2011/12/21 Uwe Kleine-K?nig <[email protected]>:

Quoted 1 line

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.

Quoted 5 lines · Ohad Ben-Cohen

Hi Uwe,

2011/12/21 Uwe Kleine-K?nig <[email protected]>:

Quoted 1 line

If you're interested in the details I can publish my tree I think.

Quoted 1 line

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.

Quoted 14 lines · Uwe Kleine-K?nig

Quoted 6 lines · Ohad Ben-Cohen

Hi Uwe,

2011/12/21 Uwe Kleine-K?nig <[email protected]>:

Quoted 1 line

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:

(quoted diff) +4 -0

+#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.)

Quoted 5 lines · Russell King - ARM Linux

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).

Quoted 11 lines · Catalin Marinas

Quoted 5 lines · Russell King - ARM Linux

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?

Quoted 6 lines · Uwe Kleine-K?nig

Quoted 5 lines · Ohad Ben-Cohen

Hi Uwe,

2011/12/21 Uwe Kleine-K?nig <[email protected]>:

Quoted 1 line

If you're interested in the details I can publish my tree I think.

Quoted 8 lines

Quoted 1 line

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++) {

(quoted diff) +1 -3

-               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:

(quoted diff) +4 -0

+#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":

(quoted diff) +3 -3

-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"

arch/arm/mm/Kconfig +1 -1

-       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.)