[RFC PATCH] ARM: new architecture for Energy Micro's EFM32 Cortex-M3 SoCs
Uwe Kleine-König
u.kleine-koenig at pengutronix.de
Wed Dec 21 11:44:33 EST 2011
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 at 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 at 80000000" is preferred here.
ok.
> > + 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";
> > + };
> > + };
> > +};
> > 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
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
More information about the linux-arm-kernel
mailing list