| Message ID | 1313526898-19920-3-git-send-email-robherring2@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Rob, On Tue, Aug 16, 2011 at 03:34:54PM -0500, Rob Herring wrote: > From: Rob Herring <rob.herring@calxeda.com> > > This adds basic support for the Calxeda Highbank platform. > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> > --- [...] > +static void __init highbank_timer_init(void) > +{ > + int irq; > + struct device_node *np; > + void __iomem *timer_base; > + > + /* Map system registers */ > + np = of_find_compatible_node(NULL, NULL, "calxeda,hb-sregs"); > + sregs_base = of_iomap(np, 0); Should the return values be checked here? I know that all valid device trees should have these nodes and valid a reg property, but I don't know if the error handling needs to be a bit more explicit. For my platform I have put these checks and panics() if they fail, but I'm not sure if that's the right thing! Jamie
On Tue, Aug 16, 2011 at 03:34:54PM -0500, Rob Herring wrote: > +config ARCH_HIGHBANK > + bool "Calxeda Highbank-based" > + select CPU_V7 > + select AUTO_ZRELADDR > + select ARM_PATCH_PHYS_VIRT > + select ARM_GIC > + select HAVE_ARM_SCU > + select ARM_AMBA > + select ARM_TIMER_SP804 > + select PL330 > + select CLKDEV_LOOKUP > + select GENERIC_CLOCKEVENTS > + select USE_OF > + select ARCH_WANT_OPTIONAL_GPIOLIB Please arrange these select statements in alphabetical order. > diff --git a/arch/arm/mach-highbank/include/mach/memory.h b/arch/arm/mach-highbank/include/mach/memory.h > new file mode 100644 > index 0000000..8b13789 > --- /dev/null > +++ b/arch/arm/mach-highbank/include/mach/memory.h > @@ -0,0 +1 @@ > + It's good practice to put something in a file other than a blank line.
On Tuesday 16 August 2011, Rob Herring wrote: > +void __iomem *a9_base_addr = ((void __iomem *)(HB_MPIC_VIRT_BASE)); > +void __iomem *sregs_base; > + > +static struct map_desc highbank_io_desc[] __initdata = { > + { > + .virtual = HB_MPIC_VIRT_BASE, > + .pfn = 0, /* run-time */ > + .length = SZ_4K, > + .type = MT_DEVICE, > + }, > +#ifdef CONFIG_DEBUG_LL > + { > + .virtual = HB_DEBUG_LL_VIRT_BASE, > + .pfn = __phys_to_pfn(HB_DEBUG_LL_PHYS_BASE), > + .length = SZ_4K, > + .type = MT_DEVICE, > + }, > +#endif > +}; > + > +static void __init highbank_map_io(void) > +{ > + unsigned long base; > + > + /* Get SCU base */ > + asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base)); > + > + highbank_io_desc[0].pfn = __phys_to_pfn(base); > + iotable_init(highbank_io_desc, ARRAY_SIZE(highbank_io_desc)); > +} I really liked the way that Barry moved the io_desc out to the drivers using them, e.g arch/arm/mach-prima2/lluart.c. Can you do the same thing with your lluart and with the a9_base_addr? I guess it can live locally in platsmp.c. > +void highbank_init_irq(void) > +{ > + struct device_node *node; > + struct of_intc_desc desc; > + int n = 0; > + > + memset(&desc, 0, sizeof(desc)); > + desc.controller = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic"); > + gic_of_init(&desc); > + node = desc.controller; > + for_each_child_of_node(node, desc.controller) { > + gic_of_ppi_init(&desc); > + } > + > + for_each_compatible_node(node, NULL, "arm,pl061") { > + irq_domain_add_simple(node, 160 + (8 * n)); > + n++; > + } Where does the "160 + (8 * n)" come from? Is that something that should be in a property of the gic binding? > +#ifdef CONFIG_CACHE_L2X0 > + l2x0_of_init(0, ~0UL); > +#endif > +} Hmm, I missed that during the review of the patch that adds l2x0_of_init, but I think the #ifdef should really be in the header file, not in the user, so that calling l2x0_of_init when CONFIG_CACHE_L2X0 is not set automatically turns into an empty stub. > +static void __init highbank_timer_init(void) > +{ > + int irq; > + struct device_node *np; > + void __iomem *timer_base; > + > + /* Map system registers */ > + np = of_find_compatible_node(NULL, NULL, "calxeda,hb-sregs"); > + sregs_base = of_iomap(np, 0); > + > + np = of_find_compatible_node(NULL, NULL, "arm,sp804"); > + timer_base = of_iomap(np, 0); > + irq = irq_of_parse_and_map(np, 0); > + > + highbank_clocks_init(); > + > + sp804_clocksource_init(timer_base + 0x20, "timer1"); > + sp804_clockevents_init(timer_base, irq, "timer0"); > +} How about moving the sp804 initialization from device tree into the arch/arm/common/timer-sp.c file? Why do you initialize sregs_base from timer_init? > diff --git a/arch/arm/mach-highbank/include/mach/timex.h b/arch/arm/mach-highbank/include/mach/timex.h > new file mode 100644 > index 0000000..88dac7a > --- /dev/null > +++ b/arch/arm/mach-highbank/include/mach/timex.h > @@ -0,0 +1,6 @@ > +#ifndef __MACH_TIMEX_H > +#define __MACH_TIMEX_H > + > +#define CLOCK_TICK_RATE 1000000 > + > +#endif In 3.2, we shouldn't need this any more. We'll have to come up with a way to remember removing the new definitions that come in in parallel to the patch that removes the old ones. > +#ifndef _MACH_HIGHBANK__SYSREGS_H_ > +#define _MACH_HIGHBANK__SYSREGS_H_ > + > +extern void __iomem *sregs_base; > + > +#define HB_SREG_A9_PWR_REQ 0xf00 > +#define HB_SREG_A9_BOOT_STAT 0xf04 > +#define HB_SREG_A9_BOOT_DATA 0xf08 > + > +#define HB_PWR_SUSPEND 0 > +#define HB_PWR_SOFT_RESET 1 > +#define HB_PWR_HARD_RESET 2 > +#define HB_PWR_SHUTDOWN 3 > + > +#endif Do these really need to be global? I think it's better to put the base address and register definitions into a single file and export functions to be used from elsewhere. Arnd
On Thu, Aug 18, 2011 at 05:34:25PM +0200, Arnd Bergmann wrote: > On Tuesday 16 August 2011, Rob Herring wrote: > > +static void __init highbank_timer_init(void) > > +{ > > + int irq; > > + struct device_node *np; > > + void __iomem *timer_base; > > + > > + /* Map system registers */ > > + np = of_find_compatible_node(NULL, NULL, "calxeda,hb-sregs"); > > + sregs_base = of_iomap(np, 0); > > + > > + np = of_find_compatible_node(NULL, NULL, "arm,sp804"); > > + timer_base = of_iomap(np, 0); > > + irq = irq_of_parse_and_map(np, 0); > > + > > + highbank_clocks_init(); > > + > > + sp804_clocksource_init(timer_base + 0x20, "timer1"); > > + sp804_clockevents_init(timer_base, irq, "timer0"); > > +} > > How about moving the sp804 initialization from device tree into the > arch/arm/common/timer-sp.c file? > > Why do you initialize sregs_base from timer_init? That'd create special cases - ARM platforms need registers twiddled to change the clock rate for the timers from 32kHz to a more sensible 1MHz. > > diff --git a/arch/arm/mach-highbank/include/mach/timex.h b/arch/arm/mach-highbank/include/mach/timex.h > > new file mode 100644 > > index 0000000..88dac7a > > --- /dev/null > > +++ b/arch/arm/mach-highbank/include/mach/timex.h > > @@ -0,0 +1,6 @@ > > +#ifndef __MACH_TIMEX_H > > +#define __MACH_TIMEX_H > > + > > +#define CLOCK_TICK_RATE 1000000 > > + > > +#endif > > In 3.2, we shouldn't need this any more. We'll have to come up with a > way to remember removing the new definitions that come in in parallel > to the patch that removes the old ones. Has anyone really properly evaluated the CLOCK_TICK_RATE issues on things like NTP etc? I have problems with kernels on OMAP4 constantly jumping forwards/back by .5sec when NTP is running which suggests that there's something not quite right _somewhere_. Given that OMAP uses an untrue value for this, and the platforms I have which _do_ behave properly when running NTP have correct values, I _still_ remain entirely unconvinced about the claims surrounding CLOCK_TICK_RATE not mattering. Has anyone managed to run NTP on OMAP4 and had it sync successfully over a few days?
On Fri, Aug 19, 2011 at 02:43:57PM +0800, Shawn Guo wrote: > On Tue, Aug 16, 2011 at 03:34:54PM -0500, Rob Herring wrote: > > From: Rob Herring <rob.herring@calxeda.com> > > > > This adds basic support for the Calxeda Highbank platform. > > > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> [...] > > +static void __init highbank_map_io(void) > > +{ > > + unsigned long base; > > + > > + /* Get SCU base */ > > + asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base)); > > + > > + highbank_io_desc[0].pfn = __phys_to_pfn(base); > > + iotable_init(highbank_io_desc, ARRAY_SIZE(highbank_io_desc)); > > +} > > + > It's a great demo that we can get this base address from CA9 itself. > However I'm wondering if we have to do so, since this address should > be known for given SoC. Can we put this known base into device tree > and map it when needed? In that case, we can save another static > mapping. > Hmm, if CA9 can figure this address out by itself, why do not we create a init hook in arch/arm/kernel/smp_scu.c to have scu code to get and map the address? Then the parameter "scu_base" in those scu helper functions can be saved.
On Thursday 18 August 2011, Russell King - ARM Linux wrote: > On Thu, Aug 18, 2011 at 05:34:25PM +0200, Arnd Bergmann wrote: > > On Tuesday 16 August 2011, Rob Herring wrote: > > > +static void __init highbank_timer_init(void) > > > +{ > > > + int irq; > > > + struct device_node *np; > > > + void __iomem *timer_base; > > > + > > > + /* Map system registers */ > > > + np = of_find_compatible_node(NULL, NULL, "calxeda,hb-sregs"); > > > + sregs_base = of_iomap(np, 0); > > > + > > > + np = of_find_compatible_node(NULL, NULL, "arm,sp804"); > > > + timer_base = of_iomap(np, 0); > > > + irq = irq_of_parse_and_map(np, 0); > > > + > > > + highbank_clocks_init(); > > > + > > > + sp804_clocksource_init(timer_base + 0x20, "timer1"); > > > + sp804_clockevents_init(timer_base, irq, "timer0"); > > > +} > > > > How about moving the sp804 initialization from device tree into the > > arch/arm/common/timer-sp.c file? > > > > Why do you initialize sregs_base from timer_init? > > That'd create special cases - ARM platforms need registers twiddled to > change the clock rate for the timers from 32kHz to a more sensible 1MHz. Is that a bad thing? Platforms that don't need the special case can simply call sp804_clocksource_init_dt() which scans the device tree, while other platforms do whatever is necessary to the registers and then call the existing sp804_clockevents_init. > > > diff --git a/arch/arm/mach-highbank/include/mach/timex.h b/arch/arm/mach-highbank/include/mach/timex.h > > > new file mode 100644 > > > index 0000000..88dac7a > > > --- /dev/null > > > +++ b/arch/arm/mach-highbank/include/mach/timex.h > > > @@ -0,0 +1,6 @@ > > > +#ifndef __MACH_TIMEX_H > > > +#define __MACH_TIMEX_H > > > + > > > +#define CLOCK_TICK_RATE 1000000 > > > + > > > +#endif > > > > In 3.2, we shouldn't need this any more. We'll have to come up with a > > way to remember removing the new definitions that come in in parallel > > to the patch that removes the old ones. > > Has anyone really properly evaluated the CLOCK_TICK_RATE issues on things > like NTP etc? I have problems with kernels on OMAP4 constantly jumping > forwards/back by .5sec when NTP is running which suggests that there's > something not quite right _somewhere_. > > Given that OMAP uses an untrue value for this, and the platforms I have > which do behave properly when running NTP have correct values, I still > remain entirely unconvinced about the claims surrounding CLOCK_TICK_RATE > not mattering. (Taking John, Deepak an Thomas on Cc, they have all worked on this in the past) The argument why it is assumed to be safe is that almost all machines today use a totally bogus CLOCK_TICK_RATE. This includes most x86 machines (which don't use PIT for periodic ticks any more), all sparc, powerpc, s390, parisc and mips machines that have never used the PIT time base but define CLOCK_TICK_RATE to 1193180 or 1193182 anyway. The only explanation I have for these working correctly is that the effect of the ACTHZ macro is not what it was meant to be and that it should better be removed. > Has anyone managed to run NTP on OMAP4 and had it sync successfully over > a few days? Omap is weird in many ways here. They define CLOCK_TICK_RATE to be equal to HZ, which in turn is a power-of-two value, typically 128. I have verified that the strange CLOCK_TICK_RATE won't cause problems in the kernel (in theory), but I could well imagine that the problems of OMAP are stemming from rounding problems when converting between kernel ticks (128 Hz) to user ticks (100 Hz) in kernel/time.c, or perhaps from the omap read_persistent_clock() function not being SMP safe. Arnd
On Tue, Aug 16, 2011 at 03:34:54PM -0500, Rob Herring wrote: > From: Rob Herring <rob.herring@calxeda.com> > > This adds basic support for the Calxeda Highbank platform. > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> > --- [...] > + > +void highbank_init_irq(void) > +{ > + struct device_node *node; > + struct of_intc_desc desc; > + int n = 0; > + > + memset(&desc, 0, sizeof(desc)); > + desc.controller = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic"); > + gic_of_init(&desc); > + node = desc.controller; > + for_each_child_of_node(node, desc.controller) { > + gic_of_ppi_init(&desc); > + } > + > + for_each_compatible_node(node, NULL, "arm,pl061") { > + irq_domain_add_simple(node, 160 + (8 * n)); > + n++; > + } > + > +#ifdef CONFIG_CACHE_L2X0 > + l2x0_of_init(0, ~0UL); > +#endif Would it be good to create a l2x0_of_init() in cache-l2x0.h to git rid of this #ifdef?
On 08/19/2011 02:17 AM, Shawn Guo wrote: > On Fri, Aug 19, 2011 at 02:43:57PM +0800, Shawn Guo wrote: >> On Tue, Aug 16, 2011 at 03:34:54PM -0500, Rob Herring wrote: >>> From: Rob Herring <rob.herring@calxeda.com> >>> >>> This adds basic support for the Calxeda Highbank platform. >>> >>> Signed-off-by: Rob Herring <rob.herring@calxeda.com> > [...] >>> +static void __init highbank_map_io(void) >>> +{ >>> + unsigned long base; >>> + >>> + /* Get SCU base */ >>> + asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base)); >>> + >>> + highbank_io_desc[0].pfn = __phys_to_pfn(base); >>> + iotable_init(highbank_io_desc, ARRAY_SIZE(highbank_io_desc)); >>> +} >>> + >> It's a great demo that we can get this base address from CA9 itself. >> However I'm wondering if we have to do so, since this address should >> be known for given SoC. Can we put this known base into device tree >> and map it when needed? In that case, we can save another static >> mapping. >> > Hmm, if CA9 can figure this address out by itself, why do not we create > a init hook in arch/arm/kernel/smp_scu.c to have scu code to get and > map the address? Then the parameter "scu_base" in those scu helper > functions can be saved. > smp_scu.c is not CA9 only and this is a CA9 specific feature. Also, you have to statically map the scu to get the core count. ioremap is not up at that point in time. Don't be fooled by omap code either. They do an ioremap, but it handles their static mappings as well. Rob
On 08/20/2011 09:48 AM, Shawn Guo wrote: > On Tue, Aug 16, 2011 at 03:34:54PM -0500, Rob Herring wrote: >> From: Rob Herring <rob.herring@calxeda.com> >> >> This adds basic support for the Calxeda Highbank platform. >> >> Signed-off-by: Rob Herring <rob.herring@calxeda.com> >> --- > [...] >> +void highbank_init_irq(void) >> +{ >> + struct device_node *node; >> + struct of_intc_desc desc; >> + int n = 0; >> + >> + memset(&desc, 0, sizeof(desc)); >> + desc.controller = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic"); >> + gic_of_init(&desc); >> + node = desc.controller; >> + for_each_child_of_node(node, desc.controller) { >> + gic_of_ppi_init(&desc); >> + } >> + >> + for_each_compatible_node(node, NULL, "arm,pl061") { >> + irq_domain_add_simple(node, 160 + (8 * n)); >> + n++; >> + } >> + > 160, magic number? I guess it's the GIC IRQ number has been > discovered in gic_dist_init(). I'm not sure if we should simply > define a macro for it or manage to retrieve it from GIC. It's the number of GIC irqs. This is temporary until we have dynamic assignment of linux irq numbers. Rob
On 08/20/2011 11:10 AM, Shawn Guo wrote: > On Tue, Aug 16, 2011 at 03:34:54PM -0500, Rob Herring wrote: >> From: Rob Herring <rob.herring@calxeda.com> >> >> This adds basic support for the Calxeda Highbank platform. >> >> Signed-off-by: Rob Herring <rob.herring@calxeda.com> >> --- > [...] >> +DT_MACHINE_START(HIGHBANK, "Highbank") >> + .map_io = highbank_map_io, >> + .init_irq = highbank_init_irq, >> + .nr_irqs = NR_IRQS, > > Is it really needed? For single kernel binary, yes. See Nicolas Pitre's task list for mach/irqs.h. Rob > >> + .timer = &highbank_timer, >> + .init_machine = highbank_init, >> + .dt_compat = highbank_match, >> +MACHINE_END >
Arnd, On 08/18/2011 10:34 AM, Arnd Bergmann wrote: > On Tuesday 16 August 2011, Rob Herring wrote: > >> +void __iomem *a9_base_addr = ((void __iomem *)(HB_MPIC_VIRT_BASE)); >> +void __iomem *sregs_base; >> + >> +static struct map_desc highbank_io_desc[] __initdata = { >> + { >> + .virtual = HB_MPIC_VIRT_BASE, >> + .pfn = 0, /* run-time */ >> + .length = SZ_4K, >> + .type = MT_DEVICE, >> + }, >> +#ifdef CONFIG_DEBUG_LL >> + { >> + .virtual = HB_DEBUG_LL_VIRT_BASE, >> + .pfn = __phys_to_pfn(HB_DEBUG_LL_PHYS_BASE), >> + .length = SZ_4K, >> + .type = MT_DEVICE, >> + }, >> +#endif >> +}; >> + >> +static void __init highbank_map_io(void) >> +{ >> + unsigned long base; >> + >> + /* Get SCU base */ >> + asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base)); >> + >> + highbank_io_desc[0].pfn = __phys_to_pfn(base); >> + iotable_init(highbank_io_desc, ARRAY_SIZE(highbank_io_desc)); >> +} > > I really liked the way that Barry moved the io_desc out to the > drivers using them, e.g arch/arm/mach-prima2/lluart.c. > > Can you do the same thing with your lluart and with the a9_base_addr? > I guess it can live locally in platsmp.c. Okay. > >> +void highbank_init_irq(void) >> +{ >> + struct device_node *node; >> + struct of_intc_desc desc; >> + int n = 0; >> + >> + memset(&desc, 0, sizeof(desc)); >> + desc.controller = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic"); >> + gic_of_init(&desc); >> + node = desc.controller; >> + for_each_child_of_node(node, desc.controller) { >> + gic_of_ppi_init(&desc); >> + } >> + >> + for_each_compatible_node(node, NULL, "arm,pl061") { >> + irq_domain_add_simple(node, 160 + (8 * n)); >> + n++; >> + } > > Where does the "160 + (8 * n)" come from? Is that something that should > be in a property of the gic binding? All this should go away once we have dynamic linux irq number assignment. Actually, I should just delete this for now as the pl061 driver doesn't support interrupts yet with DT binding (without platform data). > >> +#ifdef CONFIG_CACHE_L2X0 >> + l2x0_of_init(0, ~0UL); >> +#endif >> +} > > Hmm, I missed that during the review of the patch that adds l2x0_of_init, > but I think the #ifdef should really be in the header file, not in the > user, so that calling l2x0_of_init when CONFIG_CACHE_L2X0 is not set > automatically turns into an empty stub. It's also a problem with l2x0_init. I'll add a patch to do that. > >> +static void __init highbank_timer_init(void) >> +{ >> + int irq; >> + struct device_node *np; >> + void __iomem *timer_base; >> + >> + /* Map system registers */ >> + np = of_find_compatible_node(NULL, NULL, "calxeda,hb-sregs"); >> + sregs_base = of_iomap(np, 0); >> + >> + np = of_find_compatible_node(NULL, NULL, "arm,sp804"); >> + timer_base = of_iomap(np, 0); >> + irq = irq_of_parse_and_map(np, 0); >> + >> + highbank_clocks_init(); >> + >> + sp804_clocksource_init(timer_base + 0x20, "timer1"); >> + sp804_clockevents_init(timer_base, irq, "timer0"); >> +} > > How about moving the sp804 initialization from device tree into the > arch/arm/common/timer-sp.c file? > > Why do you initialize sregs_base from timer_init? It will be needed before the clocks are initialized. The clock code is not using it at the moment as I just did a minimal fixed clock implementation until the clock api and DT clock bindings gets sorted out. As there are multiple users, I didn't put it in highbank_clocks_init. > >> diff --git a/arch/arm/mach-highbank/include/mach/timex.h b/arch/arm/mach-highbank/include/mach/timex.h >> new file mode 100644 >> index 0000000..88dac7a >> --- /dev/null >> +++ b/arch/arm/mach-highbank/include/mach/timex.h >> @@ -0,0 +1,6 @@ >> +#ifndef __MACH_TIMEX_H >> +#define __MACH_TIMEX_H >> + >> +#define CLOCK_TICK_RATE 1000000 >> + >> +#endif > > In 3.2, we shouldn't need this any more. We'll have to come up with a > way to remember removing the new definitions that come in in parallel > to the patch that removes the old ones. I'm tracking the various clean-ups and we can coordinate the order things go in. I've already made gpio.h empty for example, so gpio will fail to compile if enabled in this series. Or I can just submit a patch deleting this file later. It will just be dead code and won't conflict. > >> +#ifndef _MACH_HIGHBANK__SYSREGS_H_ >> +#define _MACH_HIGHBANK__SYSREGS_H_ >> + >> +extern void __iomem *sregs_base; >> + >> +#define HB_SREG_A9_PWR_REQ 0xf00 >> +#define HB_SREG_A9_BOOT_STAT 0xf04 >> +#define HB_SREG_A9_BOOT_DATA 0xf08 >> + >> +#define HB_PWR_SUSPEND 0 >> +#define HB_PWR_SOFT_RESET 1 >> +#define HB_PWR_HARD_RESET 2 >> +#define HB_PWR_SHUTDOWN 3 >> + >> +#endif > > Do these really need to be global? > > I think it's better to put the base address and register definitions into a > single file and export functions to be used from elsewhere. Yes, sregs are a random collection of functions, so it's going to be a mixture of various users. Just HB_PWR_* alone are in 2 or 3 different places. Rob
Arnd, On 08/19/2011 09:11 AM, Arnd Bergmann wrote: > On Thursday 18 August 2011, Russell King - ARM Linux wrote: >> On Thu, Aug 18, 2011 at 05:34:25PM +0200, Arnd Bergmann wrote: >>> On Tuesday 16 August 2011, Rob Herring wrote: >>>> +static void __init highbank_timer_init(void) >>>> +{ >>>> + int irq; >>>> + struct device_node *np; >>>> + void __iomem *timer_base; >>>> + >>>> + /* Map system registers */ >>>> + np = of_find_compatible_node(NULL, NULL, "calxeda,hb-sregs"); >>>> + sregs_base = of_iomap(np, 0); >>>> + >>>> + np = of_find_compatible_node(NULL, NULL, "arm,sp804"); >>>> + timer_base = of_iomap(np, 0); >>>> + irq = irq_of_parse_and_map(np, 0); >>>> + >>>> + highbank_clocks_init(); >>>> + >>>> + sp804_clocksource_init(timer_base + 0x20, "timer1"); >>>> + sp804_clockevents_init(timer_base, irq, "timer0"); >>>> +} >>> >>> How about moving the sp804 initialization from device tree into the >>> arch/arm/common/timer-sp.c file? >>> >>> Why do you initialize sregs_base from timer_init? >> >> That'd create special cases - ARM platforms need registers twiddled to >> change the clock rate for the timers from 32kHz to a more sensible 1MHz. > > Is that a bad thing? Platforms that don't need the special case can > simply call sp804_clocksource_init_dt() which scans the device tree, > while other platforms do whatever is necessary to the registers > and then call the existing sp804_clockevents_init. > This was something I considered as I've worked on doing that in other cases like l2x0 and gic. There's also the issue that the clocksource and clockevent timers may or may not be at the same address range (base and base + 0x20), so you may need 1 or 2 mappings. You could figure all that out, but it would be a lot of work for little gain. Also, which timer is used for which function is also platform dependent. For example, we don't have the 2nd timer's interrupt hooked up so it has to be the . On top of that this really needs to wait until the DT clock binding is in place as clock setup is a major part of the init. Rob
On Fri, Aug 19, 2011 at 04:11:35PM +0200, Arnd Bergmann wrote: > Is that a bad thing? Platforms that don't need the special case can > simply call sp804_clocksource_init_dt() which scans the device tree, > while other platforms do whatever is necessary to the registers > and then call the existing sp804_clockevents_init. It means we're not solving the problem. We end up with some platforms which do the sp804 init from DT, others which are DT but ignore the DT stuff for sp804. That's not very consistent, and in the long run is rather confusing.
On Tue, Aug 16, 2011 at 03:34:54PM -0500, Rob Herring wrote: > From: Rob Herring <rob.herring@calxeda.com> > > This adds basic support for the Calxeda Highbank platform. > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> > --- [...] > diff --git a/arch/arm/mach-highbank/include/mach/entry-macro.S b/arch/arm/mach-highbank/include/mach/entry-macro.S > new file mode 100644 > index 0000000..56be409 > --- /dev/null > +++ b/arch/arm/mach-highbank/include/mach/entry-macro.S > @@ -0,0 +1,9 @@ > +#include <asm/hardware/gic.h> > +#include <asm/hardware/entry-macro-gic.S> It seems that the second file already includes the first one. > + > + .macro disable_fiq > + .endm > + > + .macro arch_ret_to_user, tmp1, tmp2 > + .endm > +
On Tue, Aug 16, 2011 at 03:34:54PM -0500, Rob Herring wrote: > From: Rob Herring <rob.herring@calxeda.com> > > This adds basic support for the Calxeda Highbank platform. > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> > --- [...] > +void highbank_init_irq(void) > +{ > + struct device_node *node; > + struct of_intc_desc desc; > + int n = 0; > + > + memset(&desc, 0, sizeof(desc)); > + desc.controller = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic"); > + gic_of_init(&desc); > + node = desc.controller; > + for_each_child_of_node(node, desc.controller) { > + gic_of_ppi_init(&desc); > + } > + I failed to find the implementation of gic_of_ppi_init() in your patch series below. [RFC PATCH 0/3] Yet another GIC OF binding series Also one nit: the braces seem redundant.
On Mon, Aug 22, 2011 at 04:35:26PM +0800, Shawn Guo wrote: > On Tue, Aug 16, 2011 at 03:34:54PM -0500, Rob Herring wrote: > > From: Rob Herring <rob.herring@calxeda.com> > > > > This adds basic support for the Calxeda Highbank platform. > > > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> > > --- > [...] > > +void highbank_init_irq(void) > > +{ > > + struct device_node *node; > > + struct of_intc_desc desc; > > + int n = 0; > > + > > + memset(&desc, 0, sizeof(desc)); > > + desc.controller = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic"); > > + gic_of_init(&desc); > > + node = desc.controller; > > + for_each_child_of_node(node, desc.controller) { > > + gic_of_ppi_init(&desc); > > + } > > + > I failed to find the implementation of gic_of_ppi_init() in your patch > series below. > > [RFC PATCH 0/3] Yet another GIC OF binding series > I just found it in another private post you made to Marc. (Marc made it public with his reply) > Also one nit: the braces seem redundant. >
Hi Shawn, On Mon, Aug 22, 2011 at 01:55:36PM +0800, Shawn Guo wrote: > On Tue, Aug 16, 2011 at 03:34:54PM -0500, Rob Herring wrote: > > From: Rob Herring <rob.herring@calxeda.com> > > > > This adds basic support for the Calxeda Highbank platform. > > > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> > > --- > [...] > > diff --git a/arch/arm/mach-highbank/include/mach/entry-macro.S b/arch/arm/mach-highbank/include/mach/entry-macro.S > > new file mode 100644 > > index 0000000..56be409 > > --- /dev/null > > +++ b/arch/arm/mach-highbank/include/mach/entry-macro.S > > @@ -0,0 +1,9 @@ > > +#include <asm/hardware/gic.h> > > +#include <asm/hardware/entry-macro-gic.S> > > It seems that the second file already includes the first one. It's preferred to use explicit includes. From Documentation/SubmitChecklist: 1: If you use a facility then #include the file that defines/declares that facility. Don't depend on other header files pulling in ones that you use. Jamie
Shawn, On 08/22/2011 03:35 AM, Shawn Guo wrote: > On Tue, Aug 16, 2011 at 03:34:54PM -0500, Rob Herring wrote: >> From: Rob Herring <rob.herring@calxeda.com> >> >> This adds basic support for the Calxeda Highbank platform. >> >> Signed-off-by: Rob Herring <rob.herring@calxeda.com> >> --- > [...] >> +void highbank_init_irq(void) >> +{ >> + struct device_node *node; >> + struct of_intc_desc desc; >> + int n = 0; >> + >> + memset(&desc, 0, sizeof(desc)); >> + desc.controller = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic"); >> + gic_of_init(&desc); >> + node = desc.controller; >> + for_each_child_of_node(node, desc.controller) { >> + gic_of_ppi_init(&desc); >> + } >> + > I failed to find the implementation of gic_of_ppi_init() in your patch > series below. I sent it to the list, but it didn't seem to make it. I'll resend the series. > > [RFC PATCH 0/3] Yet another GIC OF binding series > > Also one nit: the braces seem redundant. Left over from some printks... Rob
Hi Jamie, On Mon, Aug 22, 2011 at 11:01:22AM +0100, Jamie Iles wrote: > Hi Shawn, > > On Mon, Aug 22, 2011 at 01:55:36PM +0800, Shawn Guo wrote: > > On Tue, Aug 16, 2011 at 03:34:54PM -0500, Rob Herring wrote: > > > From: Rob Herring <rob.herring@calxeda.com> > > > > > > This adds basic support for the Calxeda Highbank platform. > > > > > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> > > > --- > > [...] > > > diff --git a/arch/arm/mach-highbank/include/mach/entry-macro.S b/arch/arm/mach-highbank/include/mach/entry-macro.S > > > new file mode 100644 > > > index 0000000..56be409 > > > --- /dev/null > > > +++ b/arch/arm/mach-highbank/include/mach/entry-macro.S > > > @@ -0,0 +1,9 @@ > > > +#include <asm/hardware/gic.h> > > > +#include <asm/hardware/entry-macro-gic.S> > > > > It seems that the second file already includes the first one. > > It's preferred to use explicit includes. From > Documentation/SubmitChecklist: > > 1: If you use a facility then #include the file that > defines/declares that facility. Don't depend on other header > files pulling in ones that you use. > We should not include it even with above point, because we do not have anything in entry-macro.S requiring gic.h.
Jamie, On 08/16/2011 05:19 PM, Jamie Iles wrote: > Hi Rob, > > On Tue, Aug 16, 2011 at 03:34:54PM -0500, Rob Herring wrote: >> From: Rob Herring <rob.herring@calxeda.com> >> >> This adds basic support for the Calxeda Highbank platform. >> >> Signed-off-by: Rob Herring <rob.herring@calxeda.com> >> --- > [...] >> +static void __init highbank_timer_init(void) >> +{ >> + int irq; >> + struct device_node *np; >> + void __iomem *timer_base; >> + >> + /* Map system registers */ >> + np = of_find_compatible_node(NULL, NULL, "calxeda,hb-sregs"); >> + sregs_base = of_iomap(np, 0); > > Should the return values be checked here? I know that all valid device trees > should have these nodes and valid a reg property, but I don't know if the > error handling needs to be a bit more explicit. For my platform I have put > these checks and panics() if they fail, but I'm not sure if that's the right > thing! > A panic will stop the boot at a point the console is not up unless DEBUG_LL is enabled. If you continue, you may be able to continue long enough to get a console and then fail when something that depends on this is used. For this case, it would be when the clocks are not setup correctly (once real clock setup is implemented). As it is now, these registers aren't accessed until you do suspend, hotplug, or poweroff. So probably just a WARN_ON would be better here. Rob
Arnd, On 08/20/2011 01:44 PM, Rob Herring wrote: > Arnd, > > On 08/18/2011 10:34 AM, Arnd Bergmann wrote: >> On Tuesday 16 August 2011, Rob Herring wrote: >> >>> +void __iomem *a9_base_addr = ((void __iomem *)(HB_MPIC_VIRT_BASE)); >>> +void __iomem *sregs_base; >>> + >>> +static struct map_desc highbank_io_desc[] __initdata = { >>> + { >>> + .virtual = HB_MPIC_VIRT_BASE, >>> + .pfn = 0, /* run-time */ >>> + .length = SZ_4K, >>> + .type = MT_DEVICE, >>> + }, >>> +#ifdef CONFIG_DEBUG_LL >>> + { >>> + .virtual = HB_DEBUG_LL_VIRT_BASE, >>> + .pfn = __phys_to_pfn(HB_DEBUG_LL_PHYS_BASE), >>> + .length = SZ_4K, >>> + .type = MT_DEVICE, >>> + }, >>> +#endif >>> +}; >>> + >>> +static void __init highbank_map_io(void) >>> +{ >>> + unsigned long base; >>> + >>> + /* Get SCU base */ >>> + asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base)); >>> + >>> + highbank_io_desc[0].pfn = __phys_to_pfn(base); >>> + iotable_init(highbank_io_desc, ARRAY_SIZE(highbank_io_desc)); >>> +} >> >> I really liked the way that Barry moved the io_desc out to the >> drivers using them, e.g arch/arm/mach-prima2/lluart.c. >> >> Can you do the same thing with your lluart and with the a9_base_addr? >> I guess it can live locally in platsmp.c. > > Okay. Looking at this some more, it doesn't work too well. platsmp.c depends on CONFIG_SMP, but the SCU mapping is always needed even for !SMP because the SCU has a power mode register for each core used by the power controller. So putting it in platsmp.c would add ifdefs. So I'll move out the lluart mapping, but keep SCU mapping in highbank.c. Rob > >> >>> +void highbank_init_irq(void) >>> +{ >>> + struct device_node *node; >>> + struct of_intc_desc desc; >>> + int n = 0; >>> + >>> + memset(&desc, 0, sizeof(desc)); >>> + desc.controller = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic"); >>> + gic_of_init(&desc); >>> + node = desc.controller; >>> + for_each_child_of_node(node, desc.controller) { >>> + gic_of_ppi_init(&desc); >>> + } >>> + >>> + for_each_compatible_node(node, NULL, "arm,pl061") { >>> + irq_domain_add_simple(node, 160 + (8 * n)); >>> + n++; >>> + } >> >> Where does the "160 + (8 * n)" come from? Is that something that should >> be in a property of the gic binding? > > All this should go away once we have dynamic linux irq number > assignment. Actually, I should just delete this for now as the pl061 > driver doesn't support interrupts yet with DT binding (without platform > data). > >> >>> +#ifdef CONFIG_CACHE_L2X0 >>> + l2x0_of_init(0, ~0UL); >>> +#endif >>> +} >> >> Hmm, I missed that during the review of the patch that adds l2x0_of_init, >> but I think the #ifdef should really be in the header file, not in the >> user, so that calling l2x0_of_init when CONFIG_CACHE_L2X0 is not set >> automatically turns into an empty stub. > > It's also a problem with l2x0_init. I'll add a patch to do that. > >> >>> +static void __init highbank_timer_init(void) >>> +{ >>> + int irq; >>> + struct device_node *np; >>> + void __iomem *timer_base; >>> + >>> + /* Map system registers */ >>> + np = of_find_compatible_node(NULL, NULL, "calxeda,hb-sregs"); >>> + sregs_base = of_iomap(np, 0); >>> + >>> + np = of_find_compatible_node(NULL, NULL, "arm,sp804"); >>> + timer_base = of_iomap(np, 0); >>> + irq = irq_of_parse_and_map(np, 0); >>> + >>> + highbank_clocks_init(); >>> + >>> + sp804_clocksource_init(timer_base + 0x20, "timer1"); >>> + sp804_clockevents_init(timer_base, irq, "timer0"); >>> +} >> >> How about moving the sp804 initialization from device tree into the >> arch/arm/common/timer-sp.c file? >> >> Why do you initialize sregs_base from timer_init? > > It will be needed before the clocks are initialized. The clock code is > not using it at the moment as I just did a minimal fixed clock > implementation until the clock api and DT clock bindings gets sorted > out. As there are multiple users, I didn't put it in highbank_clocks_init. > >> >>> diff --git a/arch/arm/mach-highbank/include/mach/timex.h b/arch/arm/mach-highbank/include/mach/timex.h >>> new file mode 100644 >>> index 0000000..88dac7a >>> --- /dev/null >>> +++ b/arch/arm/mach-highbank/include/mach/timex.h >>> @@ -0,0 +1,6 @@ >>> +#ifndef __MACH_TIMEX_H >>> +#define __MACH_TIMEX_H >>> + >>> +#define CLOCK_TICK_RATE 1000000 >>> + >>> +#endif >> >> In 3.2, we shouldn't need this any more. We'll have to come up with a >> way to remember removing the new definitions that come in in parallel >> to the patch that removes the old ones. > > I'm tracking the various clean-ups and we can coordinate the order > things go in. I've already made gpio.h empty for example, so gpio will > fail to compile if enabled in this series. > > Or I can just submit a patch deleting this file later. It will just be > dead code and won't conflict. > >> >>> +#ifndef _MACH_HIGHBANK__SYSREGS_H_ >>> +#define _MACH_HIGHBANK__SYSREGS_H_ >>> + >>> +extern void __iomem *sregs_base; >>> + >>> +#define HB_SREG_A9_PWR_REQ 0xf00 >>> +#define HB_SREG_A9_BOOT_STAT 0xf04 >>> +#define HB_SREG_A9_BOOT_DATA 0xf08 >>> + >>> +#define HB_PWR_SUSPEND 0 >>> +#define HB_PWR_SOFT_RESET 1 >>> +#define HB_PWR_HARD_RESET 2 >>> +#define HB_PWR_SHUTDOWN 3 >>> + >>> +#endif >> >> Do these really need to be global? >> >> I think it's better to put the base address and register definitions into a >> single file and export functions to be used from elsewhere. > > Yes, sregs are a random collection of functions, so it's going to be a > mixture of various users. Just HB_PWR_* alone are in 2 or 3 different > places. > > Rob
On Wed, Aug 24, 2011 at 09:45:28PM -0500, Rob Herring wrote: > Arnd, > > On 08/20/2011 01:44 PM, Rob Herring wrote: > > Arnd, > > > > On 08/18/2011 10:34 AM, Arnd Bergmann wrote: > >> On Tuesday 16 August 2011, Rob Herring wrote: > >> > >>> +void __iomem *a9_base_addr = ((void __iomem *)(HB_MPIC_VIRT_BASE)); > >>> +void __iomem *sregs_base; > >>> + > >>> +static struct map_desc highbank_io_desc[] __initdata = { > >>> + { > >>> + .virtual = HB_MPIC_VIRT_BASE, > >>> + .pfn = 0, /* run-time */ > >>> + .length = SZ_4K, > >>> + .type = MT_DEVICE, > >>> + }, > >>> +#ifdef CONFIG_DEBUG_LL > >>> + { > >>> + .virtual = HB_DEBUG_LL_VIRT_BASE, > >>> + .pfn = __phys_to_pfn(HB_DEBUG_LL_PHYS_BASE), > >>> + .length = SZ_4K, > >>> + .type = MT_DEVICE, > >>> + }, > >>> +#endif > >>> +}; > >>> + > >>> +static void __init highbank_map_io(void) > >>> +{ > >>> + unsigned long base; > >>> + > >>> + /* Get SCU base */ > >>> + asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base)); > >>> + > >>> + highbank_io_desc[0].pfn = __phys_to_pfn(base); > >>> + iotable_init(highbank_io_desc, ARRAY_SIZE(highbank_io_desc)); > >>> +} > >> > >> I really liked the way that Barry moved the io_desc out to the > >> drivers using them, e.g arch/arm/mach-prima2/lluart.c. > >> > >> Can you do the same thing with your lluart and with the a9_base_addr? > >> I guess it can live locally in platsmp.c. > > > > Okay. > > Looking at this some more, it doesn't work too well. platsmp.c depends > on CONFIG_SMP, but the SCU mapping is always needed even for !SMP > because the SCU has a power mode register for each core used by the > power controller. So putting it in platsmp.c would add ifdefs. > It sounds you have a well designed power controller which will shut down different CA-9 power domains per SCU power mode settings, e.g. it will shut off different set of CA-9 power domains for Dormant and Shutdown modes. Is that the case? i.MX6Q is different from that case, so I can actually follow Arnd's suggestion to keep scu_base locally in platsmp.c.
On Thursday 25 August 2011, Rob Herring wrote: > Looking at this some more, it doesn't work too well. platsmp.c depends > on CONFIG_SMP, but the SCU mapping is always needed even for !SMP > because the SCU has a power mode register for each core used by the > power controller. So putting it in platsmp.c would add ifdefs. > > So I'll move out the lluart mapping, but keep SCU mapping in highbank.c. > Ok, makes sense. Another alternative would be to have multiple mappings for this: one local to the platsmp file and another one local to the power management code. However, that is ugly for other reasons. Arnd
On Saturday 20 August 2011, Rob Herring wrote: > >> +#ifndef _MACH_HIGHBANK__SYSREGS_H_ > >> +#define _MACH_HIGHBANK__SYSREGS_H_ > >> + > >> +extern void __iomem *sregs_base; > >> + > >> +#define HB_SREG_A9_PWR_REQ 0xf00 > >> +#define HB_SREG_A9_BOOT_STAT 0xf04 > >> +#define HB_SREG_A9_BOOT_DATA 0xf08 > >> + > >> +#define HB_PWR_SUSPEND 0 > >> +#define HB_PWR_SOFT_RESET 1 > >> +#define HB_PWR_HARD_RESET 2 > >> +#define HB_PWR_SHUTDOWN 3 > >> + > >> +#endif > > > > Do these really need to be global? > > > > I think it's better to put the base address and register definitions into a > > single file and export functions to be used from elsewhere. > > Yes, sregs are a random collection of functions, so it's going to be a > mixture of various users. Just HB_PWR_* alone are in 2 or 3 different > places. Sorry, I'm not following. Do you mean 'yes, they need to be global' or 'yes, it's better to export the functions'? Arnd
On 08/25/2011 11:02 AM, Arnd Bergmann wrote: > On Saturday 20 August 2011, Rob Herring wrote: >>>> +#ifndef _MACH_HIGHBANK__SYSREGS_H_ >>>> +#define _MACH_HIGHBANK__SYSREGS_H_ >>>> + >>>> +extern void __iomem *sregs_base; >>>> + >>>> +#define HB_SREG_A9_PWR_REQ 0xf00 >>>> +#define HB_SREG_A9_BOOT_STAT 0xf04 >>>> +#define HB_SREG_A9_BOOT_DATA 0xf08 >>>> + >>>> +#define HB_PWR_SUSPEND 0 >>>> +#define HB_PWR_SOFT_RESET 1 >>>> +#define HB_PWR_HARD_RESET 2 >>>> +#define HB_PWR_SHUTDOWN 3 >>>> + >>>> +#endif >>> >>> Do these really need to be global? >>> >>> I think it's better to put the base address and register definitions into a >>> single file and export functions to be used from elsewhere. >> >> Yes, sregs are a random collection of functions, so it's going to be a >> mixture of various users. Just HB_PWR_* alone are in 2 or 3 different >> places. > > Sorry, I'm not following. > > Do you mean 'yes, they need to be global' or 'yes, it's better to export > the functions'? > I meant the former and functions as in h/w functionality, not C functions. This will mainly be clock control plus a few other things. Is it really desired to add another layer here when these are all just single register writes? Rob
On Thursday 25 August 2011 13:03:24 Rob Herring wrote: > > > > Do you mean 'yes, they need to be global' or 'yes, it's better to export > > the functions'? > > > > I meant the former and functions as in h/w functionality, not C functions. > > This will mainly be clock control plus a few other things. Is it really > desired to add another layer here when these are all just single > register writes? It really depends on what the registers do, which I haven't seen. My feeling is still that you should have an abstract interface for drivers to use, but if the drivers need to do very little, exporting the symbol for the base address and using it in inline functions from the header could be appropriate. Arnd
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 5ebc5d9..eecee3d 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -318,6 +318,23 @@ config ARCH_BCMRING help Support for Broadcom's BCMRing platform. +config ARCH_HIGHBANK + bool "Calxeda Highbank-based" + select CPU_V7 + select AUTO_ZRELADDR + select ARM_PATCH_PHYS_VIRT + select ARM_GIC + select HAVE_ARM_SCU + select ARM_AMBA + select ARM_TIMER_SP804 + select PL330 + select CLKDEV_LOOKUP + select GENERIC_CLOCKEVENTS + select USE_OF + select ARCH_WANT_OPTIONAL_GPIOLIB + help + Support for the Calxeda Highbank SoC based boards. + config ARCH_CLPS711X bool "Cirrus Logic CLPS711x/EP721x-based" select CPU_ARM720T diff --git a/arch/arm/Makefile b/arch/arm/Makefile index 70c424e..451097e 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -141,6 +141,7 @@ machine-$(CONFIG_ARCH_EBSA110) := ebsa110 machine-$(CONFIG_ARCH_EP93XX) := ep93xx machine-$(CONFIG_ARCH_GEMINI) := gemini machine-$(CONFIG_ARCH_H720X) := h720x +machine-$(CONFIG_ARCH_HIGHBANK) := highbank machine-$(CONFIG_ARCH_INTEGRATOR) := integrator machine-$(CONFIG_ARCH_IOP13XX) := iop13xx machine-$(CONFIG_ARCH_IOP32X) := iop32x diff --git a/arch/arm/mach-highbank/Makefile b/arch/arm/mach-highbank/Makefile new file mode 100644 index 0000000..b5de3b9 --- /dev/null +++ b/arch/arm/mach-highbank/Makefile @@ -0,0 +1 @@ +obj-y := clock.o highbank.o system.o diff --git a/arch/arm/mach-highbank/Makefile.boot b/arch/arm/mach-highbank/Makefile.boot new file mode 100644 index 0000000..dae9661 --- /dev/null +++ b/arch/arm/mach-highbank/Makefile.boot @@ -0,0 +1 @@ +zreladdr-y := 0x00008000 diff --git a/arch/arm/mach-highbank/clock.c b/arch/arm/mach-highbank/clock.c new file mode 100644 index 0000000..8464e14 --- /dev/null +++ b/arch/arm/mach-highbank/clock.c @@ -0,0 +1,63 @@ +/* + * Copyright 2011 Calxeda, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see <http://www.gnu.org/licenses/>. + */ +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/errno.h> +#include <linux/clk.h> +#include <linux/clkdev.h> + + +struct clk { + unsigned long rate; +}; + +int clk_enable(struct clk *clk) +{ + return 0; +} + +void clk_disable(struct clk *clk) +{} + +unsigned long clk_get_rate(struct clk *clk) +{ + return clk->rate; +} + +long clk_round_rate(struct clk *clk, unsigned long rate) +{ + return clk->rate; +} + +int clk_set_rate(struct clk *clk, unsigned long rate) +{ + return 0; +} + +static struct clk eclk = { .rate = 200000000 }; +static struct clk pclk = { .rate = 150000000 }; + +static struct clk_lookup lookups[] = { + { .clk = &pclk, .con_id = "apb_pclk", }, + { .clk = &pclk, .dev_id = "sp804", }, + { .clk = &eclk, .dev_id = "ffe0e000.sdhci", }, + { .clk = &pclk, .dev_id = "fff36000.serial", }, +}; + +void __init highbank_clocks_init(void) +{ + clkdev_add_table(lookups, ARRAY_SIZE(lookups)); +} diff --git a/arch/arm/mach-highbank/core.h b/arch/arm/mach-highbank/core.h new file mode 100644 index 0000000..b5aa45f --- /dev/null +++ b/arch/arm/mach-highbank/core.h @@ -0,0 +1,3 @@ +extern void highbank_set_cpu_jump(int cpu, void *jump_addr); +extern void __iomem *a9_base_addr; +extern void highbank_clocks_init(void); diff --git a/arch/arm/mach-highbank/highbank.c b/arch/arm/mach-highbank/highbank.c new file mode 100644 index 0000000..49bbd13 --- /dev/null +++ b/arch/arm/mach-highbank/highbank.c @@ -0,0 +1,164 @@ +/* + * Copyright 2010-2011 Calxeda, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see <http://www.gnu.org/licenses/>. + */ +#include <linux/clk.h> +#include <linux/clkdev.h> +#include <linux/irq.h> +#include <linux/irqdomain.h> +#include <linux/of.h> +#include <linux/of_irq.h> +#include <linux/of_platform.h> +#include <linux/of_address.h> + +#include <asm/cacheflush.h> +#include <asm/unified.h> +#include <asm/smp_scu.h> +#include <asm/hardware/arm_timer.h> +#include <asm/hardware/timer-sp.h> +#include <asm/hardware/gic.h> +#include <asm/hardware/cache-l2x0.h> +#include <asm/mach/arch.h> +#include <asm/mach/map.h> +#include <asm/mach/time.h> +#include <mach/irqs.h> + +#include "core.h" +#include "sysregs.h" + +#define HB_DEBUG_LL_PHYS_BASE 0xfff36000 + +/* Static virtual mappings */ +#define HB_DEBUG_LL_VIRT_BASE 0xfee36000 +#define HB_MPIC_VIRT_BASE 0xfee00000 + +void __iomem *a9_base_addr = ((void __iomem *)(HB_MPIC_VIRT_BASE)); +void __iomem *sregs_base; + +static struct map_desc highbank_io_desc[] __initdata = { + { + .virtual = HB_MPIC_VIRT_BASE, + .pfn = 0, /* run-time */ + .length = SZ_4K, + .type = MT_DEVICE, + }, +#ifdef CONFIG_DEBUG_LL + { + .virtual = HB_DEBUG_LL_VIRT_BASE, + .pfn = __phys_to_pfn(HB_DEBUG_LL_PHYS_BASE), + .length = SZ_4K, + .type = MT_DEVICE, + }, +#endif +}; + +static void __init highbank_map_io(void) +{ + unsigned long base; + + /* Get SCU base */ + asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base)); + + highbank_io_desc[0].pfn = __phys_to_pfn(base); + iotable_init(highbank_io_desc, ARRAY_SIZE(highbank_io_desc)); +} + +#define HB_JUMP_TABLE_PHYS(cpu) (0x40 + (0x10 * (cpu))) +#define HB_JUMP_TABLE_VIRT(cpu) phys_to_virt(HB_JUMP_TABLE_PHYS(cpu)) + +void highbank_set_cpu_jump(int cpu, void *jump_addr) +{ + writel(BSYM(virt_to_phys(jump_addr)), HB_JUMP_TABLE_VIRT(cpu)); + __cpuc_flush_dcache_area(HB_JUMP_TABLE_VIRT(cpu), 16); + outer_clean_range(HB_JUMP_TABLE_PHYS(cpu), + HB_JUMP_TABLE_PHYS(cpu) + 15); +} + +void highbank_init_irq(void) +{ + struct device_node *node; + struct of_intc_desc desc; + int n = 0; + + memset(&desc, 0, sizeof(desc)); + desc.controller = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic"); + gic_of_init(&desc); + node = desc.controller; + for_each_child_of_node(node, desc.controller) { + gic_of_ppi_init(&desc); + } + + for_each_compatible_node(node, NULL, "arm,pl061") { + irq_domain_add_simple(node, 160 + (8 * n)); + n++; + } + +#ifdef CONFIG_CACHE_L2X0 + l2x0_of_init(0, ~0UL); +#endif +} + +static void __init highbank_timer_init(void) +{ + int irq; + struct device_node *np; + void __iomem *timer_base; + + /* Map system registers */ + np = of_find_compatible_node(NULL, NULL, "calxeda,hb-sregs"); + sregs_base = of_iomap(np, 0); + + np = of_find_compatible_node(NULL, NULL, "arm,sp804"); + timer_base = of_iomap(np, 0); + irq = irq_of_parse_and_map(np, 0); + + highbank_clocks_init(); + + sp804_clocksource_init(timer_base + 0x20, "timer1"); + sp804_clockevents_init(timer_base, irq, "timer0"); +} + +static struct sys_timer highbank_timer = { + .init = highbank_timer_init, +}; + +static void highbank_power_off(void) +{ + writel(HB_PWR_SHUTDOWN, sregs_base + HB_SREG_A9_PWR_REQ); + scu_power_mode(a9_base_addr, SCU_PM_POWEROFF); + + while (1) + cpu_do_idle(); +} + +static void __init highbank_init(void) +{ + pm_power_off = highbank_power_off; + + of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); +} + +static const char *highbank_match[] __initconst = { + "calxeda,highbank", + NULL, +}; + +DT_MACHINE_START(HIGHBANK, "Highbank") + .map_io = highbank_map_io, + .init_irq = highbank_init_irq, + .nr_irqs = NR_IRQS, + .timer = &highbank_timer, + .init_machine = highbank_init, + .dt_compat = highbank_match, +MACHINE_END diff --git a/arch/arm/mach-highbank/include/mach/debug-macro.S b/arch/arm/mach-highbank/include/mach/debug-macro.S new file mode 100644 index 0000000..f56096f --- /dev/null +++ b/arch/arm/mach-highbank/include/mach/debug-macro.S @@ -0,0 +1,20 @@ +/* + * Debugging macro include header + * + * Copyright (C) 1994-1999 Russell King + * Moved from linux/arch/arm/kernel/debug.S by Ben Dooks + * + * 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,rp,rv + movw \rv, #0x6000 + movt \rv, #0xfee3 + movw \rp, #0x6000 + movt \rp, #0xfff3 + .endm + +#include <asm/hardware/debug-pl01x.S> + diff --git a/arch/arm/mach-highbank/include/mach/entry-macro.S b/arch/arm/mach-highbank/include/mach/entry-macro.S new file mode 100644 index 0000000..56be409 --- /dev/null +++ b/arch/arm/mach-highbank/include/mach/entry-macro.S @@ -0,0 +1,9 @@ +#include <asm/hardware/gic.h> +#include <asm/hardware/entry-macro-gic.S> + + .macro disable_fiq + .endm + + .macro arch_ret_to_user, tmp1, tmp2 + .endm + diff --git a/arch/arm/mach-highbank/include/mach/gpio.h b/arch/arm/mach-highbank/include/mach/gpio.h new file mode 100644 index 0000000..40a8c17 --- /dev/null +++ b/arch/arm/mach-highbank/include/mach/gpio.h @@ -0,0 +1 @@ +/* empty */ diff --git a/arch/arm/mach-highbank/include/mach/io.h b/arch/arm/mach-highbank/include/mach/io.h new file mode 100644 index 0000000..5a37da2 --- /dev/null +++ b/arch/arm/mach-highbank/include/mach/io.h @@ -0,0 +1,8 @@ +#ifndef __MACH_IO_H +#define __MACH_IO_H + +#define IO_SPACE_LIMIT 0 +#define __io(a) ((void __iomem *)0) +#define __mem_pci(a) (a) + +#endif diff --git a/arch/arm/mach-highbank/include/mach/irqs.h b/arch/arm/mach-highbank/include/mach/irqs.h new file mode 100644 index 0000000..9746aab --- /dev/null +++ b/arch/arm/mach-highbank/include/mach/irqs.h @@ -0,0 +1,6 @@ +#ifndef __MACH_IRQS_H +#define __MACH_IRQS_H + +#define NR_IRQS 192 + +#endif diff --git a/arch/arm/mach-highbank/include/mach/memory.h b/arch/arm/mach-highbank/include/mach/memory.h new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/arch/arm/mach-highbank/include/mach/memory.h @@ -0,0 +1 @@ + diff --git a/arch/arm/mach-highbank/include/mach/system.h b/arch/arm/mach-highbank/include/mach/system.h new file mode 100644 index 0000000..7e81922 --- /dev/null +++ b/arch/arm/mach-highbank/include/mach/system.h @@ -0,0 +1,26 @@ +/* + * Copyright 2010-2011 Calxeda, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see <http://www.gnu.org/licenses/>. + */ +#ifndef __MACH_SYSTEM_H +#define __MACH_SYSTEM_H + +static inline void arch_idle(void) +{ + cpu_do_idle(); +} + +extern void arch_reset(char mode, const char *cmd); + +#endif diff --git a/arch/arm/mach-highbank/include/mach/timex.h b/arch/arm/mach-highbank/include/mach/timex.h new file mode 100644 index 0000000..88dac7a --- /dev/null +++ b/arch/arm/mach-highbank/include/mach/timex.h @@ -0,0 +1,6 @@ +#ifndef __MACH_TIMEX_H +#define __MACH_TIMEX_H + +#define CLOCK_TICK_RATE 1000000 + +#endif diff --git a/arch/arm/mach-highbank/include/mach/uncompress.h b/arch/arm/mach-highbank/include/mach/uncompress.h new file mode 100644 index 0000000..bbe20e6 --- /dev/null +++ b/arch/arm/mach-highbank/include/mach/uncompress.h @@ -0,0 +1,9 @@ +#ifndef __MACH_UNCOMPRESS_H +#define __MACH_UNCOMPRESS_H + +#define putc(c) +#define flush() +#define arch_decomp_setup() +#define arch_decomp_wdog() + +#endif diff --git a/arch/arm/mach-highbank/include/mach/vmalloc.h b/arch/arm/mach-highbank/include/mach/vmalloc.h new file mode 100644 index 0000000..1a3e398 --- /dev/null +++ b/arch/arm/mach-highbank/include/mach/vmalloc.h @@ -0,0 +1,6 @@ +#ifndef __MACH_VMALLOC_H +#define __MACH_VMALLOC_H + +#define VMALLOC_END 0xFEE00000UL + +#endif diff --git a/arch/arm/mach-highbank/sysregs.h b/arch/arm/mach-highbank/sysregs.h new file mode 100644 index 0000000..98ce973 --- /dev/null +++ b/arch/arm/mach-highbank/sysregs.h @@ -0,0 +1,30 @@ +/* + * Copyright 2011 Calxeda, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see <http://www.gnu.org/licenses/>. + */ +#ifndef _MACH_HIGHBANK__SYSREGS_H_ +#define _MACH_HIGHBANK__SYSREGS_H_ + +extern void __iomem *sregs_base; + +#define HB_SREG_A9_PWR_REQ 0xf00 +#define HB_SREG_A9_BOOT_STAT 0xf04 +#define HB_SREG_A9_BOOT_DATA 0xf08 + +#define HB_PWR_SUSPEND 0 +#define HB_PWR_SOFT_RESET 1 +#define HB_PWR_HARD_RESET 2 +#define HB_PWR_SHUTDOWN 3 + +#endif diff --git a/arch/arm/mach-highbank/system.c b/arch/arm/mach-highbank/system.c new file mode 100644 index 0000000..f8d1419 --- /dev/null +++ b/arch/arm/mach-highbank/system.c @@ -0,0 +1,33 @@ +/* + * Copyright 2011 Calxeda, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see <http://www.gnu.org/licenses/>. + */ +#include <linux/io.h> +#include <asm/smp_scu.h> +#include <asm/proc-fns.h> + +#include "core.h" +#include "sysregs.h" + +void arch_reset(char mode, const char *cmd) +{ + if (mode == 'h') + writel(HB_PWR_HARD_RESET, sregs_base + HB_SREG_A9_PWR_REQ); + else + writel(HB_PWR_SOFT_RESET, sregs_base + HB_SREG_A9_PWR_REQ); + + scu_power_mode(a9_base_addr, SCU_PM_POWEROFF); + cpu_do_idle(); +} + diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig index 88633fe..7d5fff7 100644 --- a/arch/arm/mm/Kconfig +++ b/arch/arm/mm/Kconfig @@ -822,7 +822,7 @@ config CACHE_L2X0 REALVIEW_EB_A9MP || SOC_IMX35 || SOC_IMX31 || MACH_REALVIEW_PBX || \ ARCH_NOMADIK || ARCH_OMAP4 || ARCH_EXYNOS4 || ARCH_TEGRA || \ ARCH_U8500 || ARCH_VEXPRESS_CA9X4 || ARCH_SHMOBILE || \ - ARCH_PRIMA2 || ARCH_ZYNQ || ARCH_CNS3XXX + ARCH_PRIMA2 || ARCH_ZYNQ || ARCH_CNS3XXX || ARCH_HIGHBANK default y select OUTER_CACHE select OUTER_CACHE_SYNC