[PATCH 2/8] arm: mach-armada: add source files
Gregory CLEMENT
gregory.clement at free-electrons.com
Tue May 15 05:59:06 EDT 2012
On 05/15/2012 11:46 AM, Ben Dooks wrote:
> On 15/05/12 09:54, Thomas Petazzoni wrote:
>> This patch adds basic source files for Marvell Armada SoCs.
>>
>> Signed-off-by: Gregory CLEMENT<gregory.clement at free-electrons.com>
>> Signed-off-by: Thomas Petazzoni<thomas.petazzoni at free-electrons.com>
>> Signed-off-by: Lior Amsalem<alior at marvell.com>
>> ---
>> arch/arm/boot/dts/a370.dtsi | 23 ++++
>> arch/arm/boot/dts/armada.dtsi | 67 ++++++++++
>> arch/arm/boot/dts/axp.dtsi | 43 +++++++
>> arch/arm/mach-armada/Kconfig | 5 +
>> arch/arm/mach-armada/Makefile | 2 +
>> arch/arm/mach-armada/Makefile.boot | 1 +
>> arch/arm/mach-armada/common.c | 56 +++++++++
>> arch/arm/mach-armada/common.h | 27 ++++
>> arch/arm/mach-armada/irq.c | 116 +++++++++++++++++
>> arch/arm/mach-armada/time.c | 243 ++++++++++++++++++++++++++++++++++++
>> 10 files changed, 583 insertions(+)
>> create mode 100644 arch/arm/boot/dts/a370.dtsi
>> create mode 100644 arch/arm/boot/dts/armada.dtsi
>> create mode 100644 arch/arm/boot/dts/axp.dtsi
>> create mode 100644 arch/arm/mach-armada/Kconfig
>> create mode 100644 arch/arm/mach-armada/Makefile
>> create mode 100644 arch/arm/mach-armada/Makefile.boot
>> create mode 100644 arch/arm/mach-armada/common.c
>> create mode 100644 arch/arm/mach-armada/common.h
>> create mode 100644 arch/arm/mach-armada/irq.c
>> create mode 100644 arch/arm/mach-armada/time.c
>>
>> diff --git a/arch/arm/boot/dts/a370.dtsi b/arch/arm/boot/dts/a370.dtsi
>> new file mode 100644
>> index 0000000..f11e56a
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/a370.dtsi
>> @@ -0,0 +1,23 @@
>> +/*
>> + * Device Tree Include file for Marvell Armada 370 family SoC
>> + *
>> + * Copyright (C) 2012 Marvell
>> + *
>> + * Lior Amsalem<alior at marvell.com>
>> + * Gregory CLEMENT<gregory.clement at free-electrons.com>
>> + * Thomas Petazzoni<thomas.petazzoni at free-electrons.com>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + *
>> + * Contains definitions specific to the Armada 370 SoC that are not
>> + * common to all Armada SoCs.
>> + */
>> +
>> +/include/ "armada.dtsi"
>
> I would use armada_xp.dtsi at the least, we've no way of knowing what
> further Armada devices are going to be produced and whether they will be
> compatible with.
>
>> +/ {
>> + model = "Marvell Armada 370 family SoC";
>> + compatible = "marvell,armada370", "marvell,armada";
>> + };
>
> Firstly, it is mrvl, not marvell everywhere else in the kernel (see
> MIPS and device tree documentation).
>
> Secondly, I would strongly advise against using the generic marketing
> name for these devices in the compatible list, "marvell,armada" as we
> have no way of knowing what new devices will come along in the future
> and if they'll be compatible.
>
>> diff --git a/arch/arm/boot/dts/armada.dtsi b/arch/arm/boot/dts/armada.dtsi
>> new file mode 100644
>> index 0000000..3c99c30
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/armada.dtsi
>> @@ -0,0 +1,67 @@
>> +/*
>> + * Device Tree Include file for Marvell Armada family SoC
>> + *
>> + * Copyright (C) 2012 Marvell
>> + *
>> + * Lior Amsalem<alior at marvell.com>
>> + * Gregory CLEMENT<gregory.clement at free-electrons.com>
>> + * Thomas Petazzoni<thomas.petazzoni at free-electrons.com>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + *
>> + * This file contains the definitions that are common to the Armada
>> + * 370 and Armada XP SoC.
>
> I really do not want to see these files in the kernel, we where suppose
> to remove any specific manchine dependency by moving to device tree and
> not push the problem from a board.c to a .dts file.
>
> Given these really should be part of the bootloader, I am not sure if a
> GPLv2 compliant license is really appropriate for these files. At best
> some form of dual-licensing to allow them to be moved out into a devtree
> repository elsewhere would be useful.
>
> (Feedback from Grant here would be useful)
>
> PS, given the amount of devtree related stuff, CC'ing Grant Likely may
> have been useful./
>
>> + */
>> +
>> +/include/ "skeleton.dtsi"
>> +
>> +/ {
>> + model = "Marvell Armada family SoC";
>> + compatible = "marvell,armada";
>> +
>> + cpus {
>> + cpu at 0 {
>> + compatible = "marvell,sheeva-v7";
>> + };
>> + };
>> +
>> + mpic: interrupt-controller at d0020000 {
>> + compatible = "marvell,mpic";
>
> Should this be "mrvl,axp_mpic" ?
>
>> + #interrupt-cells =<1>;
>> + #address-cells =<1>;
>> + #size-cells =<1>;
>> + interrupt-controller;
>> + reg =<0xd0020000 0x1000>,
>> + <0xd0021000 0x1000>;
>> + };
>> +
>> + soc {
>> + #address-cells =<1>;
>> + #size-cells =<1>;
>> + compatible = "simple-bus";
>> + interrupt-parent =<&mpic>;
>> + ranges;
>> +
>> + serial at d0012000 {
>> + compatible = "ns16550";
>
> thought these where more akin to the designware uart blocks?
>
>> + reg =<0xd0012000 0x100>;
>> + reg-shift =<2>;
>> + interrupts =<41>;
>> + };
>> + serial at d0012100 {
>> + compatible = "ns16550";
>> + reg =<0xd0012100 0x100>;
>> + reg-shift =<2>;
>> + interrupts =<42>;
>> + };
>> +
>> + timer at d0020300 {
>> + compatible = "marvell,timer";
>
> Is this really a generic marvell timer block?
>
>> + reg =<0xd0020300 0x30>;
>> + interrupts =<37>,<38>,<39>,<40>;
>> + };
>> + };
>> +};
>> +
>> diff --git a/arch/arm/boot/dts/axp.dtsi b/arch/arm/boot/dts/axp.dtsi
>> new file mode 100644
>> index 0000000..6427268
>> --- /dev/null
>
> [snip, pretty much the same comments here.
>
>> diff --git a/arch/arm/mach-armada/Kconfig b/arch/arm/mach-armada/Kconfig
>> new file mode 100644
>> index 0000000..010a6a3
>> --- /dev/null
>> +++ b/arch/arm/mach-armada/Kconfig
>> @@ -0,0 +1,5 @@
>> +if ARCH_ARMADA
>> +
>> +menu "Marvell Armada Implementations"
>> +
>> +endif
>> diff --git a/arch/arm/mach-armada/Makefile b/arch/arm/mach-armada/Makefile
>> new file mode 100644
>> index 0000000..6765961
>> --- /dev/null
>> +++ b/arch/arm/mach-armada/Makefile
>> @@ -0,0 +1,2 @@
>> +obj-y += common.o irq.o time.o
>> +
>> diff --git a/arch/arm/mach-armada/Makefile.boot b/arch/arm/mach-armada/Makefile.boot
>> new file mode 100644
>> index 0000000..b327175
>> --- /dev/null
>> +++ b/arch/arm/mach-armada/Makefile.boot
>> @@ -0,0 +1 @@
>> +zreladdr-y := 0x00008000
>> diff --git a/arch/arm/mach-armada/common.c b/arch/arm/mach-armada/common.c
>> new file mode 100644
>> index 0000000..92d9ec2
>> --- /dev/null
>> +++ b/arch/arm/mach-armada/common.c
>> @@ -0,0 +1,56 @@
>> +/*
>> + * Core functions for Marvell Armada System On Chip
>> + *
>> + * Copyright (C) 2012 Marvell
>> + *
>> + * Lior Amsalem<alior at marvell.com>
>> + * Gregory CLEMENT<gregory.clement at free-electrons.com>
>> + * Thomas Petazzoni<thomas.petazzoni at free-electrons.com>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#include<linux/kernel.h>
>> +#include<linux/delay.h>
>> +#include<linux/init.h>
>> +#include<linux/platform_device.h>
>> +#include<linux/irq.h>
>> +#include<linux/timex.h>
>> +#include<asm/page.h>
>> +#include<asm/setup.h>
>> +#include<asm/mach/map.h>
>> +#include<plat/addr-map.h>
>> +#include<mach/armada.h>
>> +#include<mach/bridge-regs.h>
>> +#include "common.h"
>
> for some reason there are no spaces between #include and the rest
> of the code?
>
>> +
>> +static struct map_desc armada_io_desc[] __initdata = {
>> + {
>> + .virtual = ARMADA_REGS_VIRT_BASE,
>> + .pfn = __phys_to_pfn(ARMADA_REGS_PHYS_BASE),
>> + .length = ARMADA_REGS_SIZE,
>> + .type = MT_DEVICE,
>> + },
>> +};
>> +
>> +void __init armada_map_io(void)
>> +{
>> + iotable_init(armada_io_desc, ARRAY_SIZE(armada_io_desc));
>> +}
>> +
>> +void armada_restart(char mode, const char *cmd)
>> +{
>> + /*
>> + * Enable soft reset to assert RSTOUTn.
>> + */
>> + writel(SOFT_RESET_OUT_EN, RSTOUTn_MASK);
>> +
>> + /*
>> + * Assert soft reset.
>> + */
>> + writel(SOFT_RESET, SYSTEM_SOFT_RESET);
>> + while (1)
>> + ;
>> +}
>> diff --git a/arch/arm/mach-armada/common.h b/arch/arm/mach-armada/common.h
>> new file mode 100644
>> index 0000000..e453161
>> --- /dev/null
>> +++ b/arch/arm/mach-armada/common.h
>> @@ -0,0 +1,27 @@
>> +/*
>> + * Core functions for Marvell Armada System On Chip
>> + *
>> + * Copyright (C) 2012 Marvell
>> + *
>> + * Lior Amsalem<alior at marvell.com>
>> + * Gregory CLEMENT<gregory.clement at free-electrons.com>
>> + * Thomas Petazzoni<thomas.petazzoni at free-electrons.com>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#ifndef __ARCH_ARMADA_COMMON_H
>> +#define __ARCH_ARMADA_COMMON_H
>> +
>> +#include<asm/exception.h>
>> +
>> +extern struct sys_timer armada_timer;
>> +
>> +void armada_map_io(void);
>> +void armada_init_irq(void);
>> +void armada_restart(char, const char *);
>> +asmlinkage void __exception_irq_entry armada_handle_irq(struct pt_regs *regs);
>> +
>> +#endif
>> diff --git a/arch/arm/mach-armada/irq.c b/arch/arm/mach-armada/irq.c
>> new file mode 100644
>> index 0000000..7006429
>> --- /dev/null
>> +++ b/arch/arm/mach-armada/irq.c
>> @@ -0,0 +1,116 @@
>> +/*
>> + * Marvall Armada SoC IRQ handling
>> + *
>> + * Copyright (C) 2012 Marvell
>> + *
>> + * Lior Amsalem<alior at marvell.com>
>> + * Gregory CLEMENT<gregory.clement at free-electrons.com>
>> + * Thomas Petazzoni<thomas.petazzoni at free-electrons.com>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#include<linux/kernel.h>
>> +#include<linux/init.h>
>> +#include<linux/irq.h>
>> +#include<linux/interrupt.h>
>> +#include<linux/io.h>
>> +#include<linux/of_address.h>
>> +#include<linux/of_irq.h>
>> +#include<linux/irqdomain.h>
>> +#include<asm/mach/arch.h>
>> +#include<asm/exception.h>
>> +
>> +/* Interrupt Controller Registers Map */
>> +#define ARMADA_INT_SET_MASK_OFFS (0xBC)
>> +#define ARMADA_INT_CLEAR_MASK_OFFS (0xB8)
>> +
>> +#define ARMADA_INT_SET_ENABLE_OFFS (0xA30)
>> +#define ARMADA_INT_CLEAR_ENABLE_OFFS (0xA34)
>> +
>> +#define ARMADA_CPU_INTACK_OFFS (0xB4)
>
> MPIC_ since you indicated earlier that it is pretty generic.
>
>> +
>> +static void __iomem *per_cpu_int_base;
>> +static void __iomem *main_int_base;
>> +static struct irq_domain *armada_mpic_domain;
>> +
>> +static void armada_irq_mask(struct irq_data *d)
>> +{
>> + writel(d->irq, per_cpu_int_base + ARMADA_INT_CLEAR_MASK_OFFS);
>> +}
>> +
>> +static void armada_irq_unmask(struct irq_data *d)
>> +{
>> + writel(d->irq, per_cpu_int_base + ARMADA_INT_SET_MASK_OFFS);
>> +}
>> +
>> +static struct irq_chip armada_irq_chip = {
>> + .name = "armada_irq",
>> + .irq_mask = armada_irq_mask,
>> + .irq_mask_ack = armada_irq_mask,
>> + .irq_unmask = armada_irq_unmask,
>> +};
>> +
>> +static int armada_mpic_irq_map(struct irq_domain *h,
>> + unsigned int virq,
>> + irq_hw_number_t hw)
>> +{
>> + armada_irq_mask(irq_get_irq_data(virq));
>> + writel(virq, main_int_base + ARMADA_INT_SET_ENABLE_OFFS);
>> +
>> + irq_set_chip_and_handler(virq,&armada_irq_chip,
>> + handle_level_irq);
>> + irq_set_status_flags(virq, IRQ_LEVEL);
>> + set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);
>> + return 0;
>
> you could have used the per-irq driver private data to keep
> things like the base of controller in.
>
>> +}
>> +
>> +static struct irq_domain_ops armada_mpic_irq_ops = {
>> + .map = armada_mpic_irq_map,
>> + .xlate = irq_domain_xlate_onecell,
>> +};
>> +
>> +static void __init armada_mpic_of_init(struct device_node *node,
>> + struct device_node *parent)
>> +{
>> + main_int_base = of_iomap(node, 0);
>> + per_cpu_int_base = of_iomap(node, 1);
>
> Please verify the return values.
>
>> +
>> + armada_mpic_domain = irq_domain_add_linear(node, NR_IRQS,
>> + &armada_mpic_irq_ops, NULL);
>> + if (!armada_mpic_domain)
>> + panic("Unable to add Armada MPIC irq domain (DT)\n");
>> +
>> + irq_set_default_host(armada_mpic_domain);
>> +}
>> +
>> +asmlinkage void __exception_irq_entry armada_handle_irq(struct pt_regs *regs)
>> +{
>> + u32 irqstat, irqnr;
>> +
>> + do {
>> + irqstat = readl_relaxed(per_cpu_int_base +
>> + ARMADA_CPU_INTACK_OFFS);
>> + irqnr = irqstat& 0x3FF;
>> +
>> + if (irqnr< 1023) {
>> + irqnr = irq_find_mapping(armada_mpic_domain, irqnr);
>> + handle_IRQ(irqnr, regs);
>> + continue;
>> + }
>> +
>> + break;
>> + } while (1);
>
> no timeout to get out of handling many IRQs?
>
>> +}
>> +
>> +static const struct of_device_id mpic_of_match[] __initconst = {
>> + { .compatible = "marvell,mpic", .data = armada_mpic_of_init },
>> + {},
>> +};
>> +
>> +void __init armada_init_irq(void)
>> +{
>> + of_irq_init(mpic_of_match);
>> +}
>> diff --git a/arch/arm/mach-armada/time.c b/arch/arm/mach-armada/time.c
>> new file mode 100644
>> index 0000000..73817ea
>> --- /dev/null
>> +++ b/arch/arm/mach-armada/time.c
>> @@ -0,0 +1,243 @@
>> +/*
>> + * Marvell Armada SoC timer handling.
>> + *
>> + * Copyright (C) 2012 Marvell
>> + *
>> + * Lior Amsalem<alior at marvell.com>
>> + * Gregory CLEMENT<gregory.clement at free-electrons.com>
>> + * Thomas Petazzoni<thomas.petazzoni at free-electrons.com>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + *
>> + * Timer 0 is used as free-running clocksource, while timer 1 is
>> + * used as clock_event_device.
>> + */
>> +
>> +#include<linux/kernel.h>
>> +#include<linux/timer.h>
>> +#include<linux/clockchips.h>
>> +#include<linux/interrupt.h>
>> +#include<linux/of.h>
>> +#include<linux/of_irq.h>
>> +#include<linux/of_address.h>
>> +#include<linux/irq.h>
>> +#include<asm/sched_clock.h>
>> +#include<asm/mach/time.h>
>
> Style issue again.
>
>> +
>> +/*
>> + * Timer block registers.
>> + */
>> +#define TIMER_CTRL_OFF 0x0000
>> +#define TIMER0_EN 0x0001
>> +#define TIMER0_RELOAD_EN 0x0002
>> +#define TIMER0_25MHZ 0x0800
>> +#define TIMER0_DIV(div) ((div)<< 19)
>> +#define TIMER1_EN 0x0004
>> +#define TIMER1_RELOAD_EN 0x0008
>> +#define TIMER1_25MHZ 0x1000
>> +#define TIMER1_DIV(div) ((div)<< 22)
>> +#define TIMER_EVENTS_STATUS 0x0004
>> +#define TIMER0_CLR_MASK (~0x1)
>> +#define TIMER1_CLR_MASK (~0x100)
>> +#define TIMER0_RELOAD_OFF 0x0010
>> +#define TIMER0_VAL_OFF 0x0014
>> +#define TIMER1_RELOAD_OFF 0x0018
>> +#define TIMER1_VAL_OFF 0x001c
>> +
>> +/* Global timers are connected to the coherency fabric clock, and the
>> + below divider reduces their incrementing frequency. */
>> +#define TIMER_DIVIDER_SHIFT 5
>> +#define TIMER_DIVIDER (1<< TIMER_DIVIDER_SHIFT)
>> +
>> +/*
>> + * SoC-specific data.
>> + */
>> +static void __iomem *timer_base;
>> +static int timer_irq;
>> +
>> +/*
>> + * Number of timer ticks per jiffy.
>> + */
>> +static u32 ticks_per_jiffy;
>> +
>> +static u32 notrace armada_read_sched_clock(void)
>> +{
>> + return ~readl(timer_base + TIMER0_VAL_OFF);
>> +}
>> +
>> +/*
>> + * Clockevent handling.
>> + */
>> +static int
>> +armada_clkevt_next_event(unsigned long delta, struct clock_event_device *dev)
>> +{
>> + unsigned long flags;
>> + u32 u;
>> +
>> + if (delta == 0)
>> + return -ETIME;
>> +
>> + local_irq_save(flags);
>> +
>> + /*
>> + * Clear clockevent timer interrupt.
>> + */
>> + writel(TIMER1_CLR_MASK, timer_base + TIMER_EVENTS_STATUS);
>> +
>> + /*
>> + * Setup new clockevent timer value.
>> + */
>> + writel(delta, timer_base + TIMER1_VAL_OFF);
>> +
>> + /*
>> + * Enable the timer.
>> + */
>> + u = readl(timer_base + TIMER_CTRL_OFF);
>> + u = ((u& ~TIMER1_RELOAD_EN) | TIMER1_EN |
>> + TIMER1_DIV(TIMER_DIVIDER_SHIFT));
>> + writel(u, timer_base + TIMER_CTRL_OFF);
>> +
>> + local_irq_restore(flags);
>> +
>> + return 0;
>
>
> you could have halved the length by just using single line comments.
Right
>
>> +}
>> +
>> +static void
>> +armada_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *dev)
>> +{
>> + unsigned long flags;
>> + u32 u;
>> +
>> + local_irq_save(flags);
>> + if (mode == CLOCK_EVT_MODE_PERIODIC) {
>> + /*
>> + * Setup timer to fire at 1/HZ intervals.
>> + */
>> + writel(ticks_per_jiffy - 1, timer_base + TIMER1_RELOAD_OFF);
>> + writel(ticks_per_jiffy - 1, timer_base + TIMER1_VAL_OFF);
>> +
>> + /*
>> + * Enable timer.
>> + */
>> + u = readl(timer_base + TIMER_CTRL_OFF);
>> +
>> + writel((u | TIMER1_EN | TIMER1_RELOAD_EN |
>> + TIMER1_DIV(TIMER_DIVIDER_SHIFT)),
>> + timer_base + TIMER_CTRL_OFF);
>> + } else {
>> + /*
>> + * Disable timer.
>> + */
>> + u = readl(timer_base + TIMER_CTRL_OFF);
>> + writel(u& ~TIMER1_EN, timer_base + TIMER_CTRL_OFF);
>> +
>> + /*
>> + * ACK pending timer interrupt.
>> + */
>> + writel(TIMER1_CLR_MASK,
>> + timer_base + TIMER_EVENTS_STATUS);
>> +
>> + }
>
> How about handling the one-shot case that you advertise below?
>
>> +static void __init armada_time_init(void)
>> +{
>> + u32 u;
>> + struct device_node *np;
>> + unsigned int timer_clk;
>> +
>> + np = of_find_compatible_node(NULL, NULL, "marvell,timer");
>> + timer_base = of_iomap(np, 0);
>> + WARN_ON(!timer_base);
>> +
>> + if (of_find_property(np, "marvell,timer-25Mhz", NULL)) {
>> + /* The fixed 25MHz timer is available so let's use it*/
>> + u = readl(timer_base + TIMER_CTRL_OFF);
>> + writel(u | TIMER0_25MHZ | TIMER1_25MHZ,
>> + timer_base + TIMER_CTRL_OFF);
>> + timer_clk = 25000000;
>> + } else {
>> + u32 clk;
>> + of_property_read_u32(np, "clock-frequency",&clk);
>> + WARN_ON(!clk);
>> + u = readl(timer_base + TIMER_CTRL_OFF);
>> + writel(u& ~(TIMER0_25MHZ | TIMER1_25MHZ),
>> + timer_base + TIMER_CTRL_OFF);
>> + timer_clk = clk / TIMER_DIVIDER;
>> + }
>
> Give we've not got clk_() support yet, should you just force 25M
> operation until clk_get_rate() is supported?
>
This code support Aramade XP _and_ Armada 370, and for Armada 370
there is not such feature available.
> [snip]
>
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
+33 602 196 044
More information about the linux-arm-kernel
mailing list