Dear Pavel Machek,
Minor ramblings below :)
> On Wed 2012-08-29 11:26:45, Tom Rini wrote:
> > On Wed, Aug 29, 2012 at 03:41:54PM +0200, Pavel Machek wrote:
> > > Hi!
> > >
> > > > > diff --git a/arch/arm/cpu/armv7/socfpga/u-boot-spl.lds
> > > > > b/arch/arm/cpu/armv7/socfpga/u-boot-spl.lds
> > > >
> > > > You should setup MEMORY declarations like the other u-boot-spl linker
> > > > scripts do so we get build-time confirmation that we haven't exceeded
> > > > our size limitations.
> > >
> > > Hmm, I tried, but I don't know socfpga memory layout by heart.
> > >
> > > Dinh, can you help here?
> >
> > I think once you get the answers you should be able to re-post the
> > series cleanly and depend on my v5 (or v6) branch. Thanks!
>
> Porting it to your v5 was easy :-). Newer patch for review is attached.
>
> I took oportunity to cleanup whitespace in
> arch/arm/cpu/armv7/omap-common/u-boot-spl.lds. Perhaps someone can
> merge that...
Argh ... what about using git send-email for the patch submission please?
NOTE: I really have a great deal of respect for Pavel, so i do have trouble
stepping on him properly during the patch review ;-)
> Thanks,
> Pavel
>
> commit a7ecaa40d9a02e84ac81da8f49d48595d7f342ad
> Author: Pavel <pa...@ucw.cz>
> Date: Thu Aug 30 01:28:00 2012 +0200
>
> Add changes for socfpga
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c5a6f2f..df48dea 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -765,6 +765,11 @@ Nagendra T S <nagen...@mistralsolutions.com>
>
> am3517_crane ARM ARMV7 (AM35x SoC)
>
> +Dinh Nguyen <dingu...@altera.com>
> +Chin Liang See <cl...@altera.com>
> +
> + socfpga socfpga_cyclone5
> +
> Kyungmin Park <kyungmin.p...@samsung.com>
>
> apollon ARM1136EJS
> diff --git a/arch/arm/cpu/armv7/omap-common/u-boot-spl.lds
> b/arch/arm/cpu/armv7/omap-common/u-boot-spl.lds index 8867e06..1d8efb2
> 100644
> --- a/arch/arm/cpu/armv7/omap-common/u-boot-spl.lds
> +++ b/arch/arm/cpu/armv7/omap-common/u-boot-spl.lds
> @@ -37,9 +37,9 @@ SECTIONS
> {
> .text :
> {
> - __start = .;
> - arch/arm/cpu/armv7/start.o (.text)
> - *(.text*)
> + __start = .;
> + arch/arm/cpu/armv7/start.o (.text)
> + *(.text*)
> } >.sram
Maybe the cleanup should simply be split into separate patch?
> . = ALIGN(4);
> diff --git a/arch/arm/cpu/armv7/socfpga/Makefile
> b/arch/arm/cpu/armv7/socfpga/Makefile new file mode 100644
> index 0000000..376a4bd
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/socfpga/Makefile
> @@ -0,0 +1,51 @@
> +#
> +# (C) Copyright 2000-2003
> +# Wolfgang Denk, DENX Software Engineering, w...@denx.de.
> +#
> +# Copyright (C) 2012 Altera Corporation <www.altera.com>
> +#
> +# See file CREDITS for list of people who contributed to this
> +# project.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 2 of
> +# the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that 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, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> +# MA 02111-1307 USA
> +#
> +
> +
> +include $(TOPDIR)/config.mk
> +
> +LIB = $(obj)lib$(SOC).o
> +
> +SOBJS := lowlevel_init.o
> +COBJS-y := misc.o timer.o
> +COBJS-$(CONFIG_SPL_BUILD) += spl.o
> +
> +COBJS := $(COBJS-y)
> +SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c)
> +OBJS := $(addprefix $(obj),$(COBJS) $(SOBJS))
> +
> +all: $(obj).depend $(LIB)
> +
> +$(LIB): $(OBJS)
> + $(call cmd_link_o_target, $(OBJS))
> +
> +#########################################################################
> +
> +# defines $(obj).depend target
> +include $(SRCTREE)/rules.mk
> +
> +sinclude $(obj).depend
> +
> +#########################################################################
> diff --git a/arch/arm/cpu/armv7/socfpga/config.mk
> b/arch/arm/cpu/armv7/socfpga/config.mk new file mode 100644
> index 0000000..b72ed1e
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/socfpga/config.mk
> @@ -0,0 +1,16 @@
> +#
> +# Copyright (C) 2011, Texas Instruments, Incorporated - http://www.ti.com/
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 2 of
> +# the License, or (at your option) any later version.
> +#
> +# This program is distributed "as is" WITHOUT ANY WARRANTY of any
> +# kind, whether express or implied; without even the implied warranty
> +# of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +ifndef CONFIG_SPL_BUILD
> +ALL-y += $(obj)u-boot.img
> +endif
> diff --git a/arch/arm/cpu/armv7/socfpga/lowlevel_init.S
> b/arch/arm/cpu/armv7/socfpga/lowlevel_init.S new file mode 100644
> index 0000000..815073e
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/socfpga/lowlevel_init.S
> @@ -0,0 +1,79 @@
> +/*
> + * Copyright (C) 2012 Altera Corporation <www.altera.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that 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 <config.h>
> +#include <version.h>
> +
> +/* Save the parameter pass in by previous boot loader */
> +.global save_boot_params
> +save_boot_params:
> + /* save the parameter here */
> +
> + /*
> + * Setup stack for exception, which is located
> + * at the end of on-chip RAM. We don't expect exception prior to
> + * relocation and if that happens, we won't worry -- it will overide
> + * global data region as the code will goto reset. After relocation,
> + * this region won't be used by other part of program.
> + * Hence it is safe.
> + */
> + ldr r0, =CONFIG_SYS_INIT_RAM_ADDR
> + ldr r1, =CONFIG_SYS_INIT_RAM_SIZE
> + add r0, r0, r1
Won't the preprocessor compute (CONFIG_SYS_INIT_RAM_ADDR +
CONFIG_SYS_INIT_RAM_SIZE) for you? Then you'd only have to do one LDR and no
ADD
instruction, saving two ticks of CPU :-)
> + ldr r1, =IRQ_STACK_START_IN
> + str r0, [r1]
> +
> + bx lr
> +
> +
> +/* Set up the platform, once the cpu has been initialized */
> +.globl lowlevel_init
> +lowlevel_init:
> +
> + /* Remap */
> +#ifdef CONFIG_SPL_BUILD
> + /*
> + * SPL : configure the remap (L3 NIC-301 GPV)
> + * so the on-chip RAM at lower memory instead ROM.
> + */
> + ldr r0, =SOCFPGA_L3REGS_ADDRESS
> + mov r1, #0x19
> + str r1, [r0]
> +#else
> + /*
> + * U-Boot : configure the remap (L3 NIC-301 GPV)
> + * so the SDRAM at lower memory instead on-chip RAM.
> + */
> + ldr r0, =SOCFPGA_L3REGS_ADDRESS
> + mov r1, #0x2
> + str r1, [r0]
> +
> + /* Private components security */
> +
> + /*
> + * U-Boot : configure private timer, global timer and cpu
> + * component access as non secure for kernel stage (as required
> + * by kernel)
> + */
> + mrc p15,4,r0,c15,c0,0
> + add r1, r0, #0x54
> + ldr r2, [r1]
> + orr r2, r2, #0xff
> + orr r2, r2, #0xf00
> + str r2, [r1]
> +#endif /* #ifdef CONFIG_SPL_BUILD */
> + mov pc, lr
> diff --git a/arch/arm/cpu/armv7/socfpga/misc.c
> b/arch/arm/cpu/armv7/socfpga/misc.c new file mode 100644
> index 0000000..b906701
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/socfpga/misc.c
> @@ -0,0 +1,54 @@
> +/*
> + * Copyright (C) 2012 Altera Corporation <www.altera.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that 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 <common.h>
> +#include <asm/io.h>
> +#include <asm/arch/reset_manager.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static const struct socfpga_reset_manager *reset_manager_base =
> + (void *)SOCFPGA_RSTMGR_ADDRESS;
> +
> +/*
> + * Write the reset manager register to cause reset
> + */
You might want to add __attribute__((noreturn)) to this function :-)
> +void reset_cpu(ulong addr)
> +{
> + /* request a warm reset */
> + writel(RSTMGR_CTRL_SWWARMRSTREQ_LSB, &reset_manager_base->ctrl);
> + /*
> + * infinite loop here as watchdog will trigger and reset
> + * the processor
> + */
> + while (1)
> + ;
> +}
[...]
> diff --git a/arch/arm/cpu/armv7/socfpga/timer.c
> b/arch/arm/cpu/armv7/socfpga/timer.c new file mode 100644
> index 0000000..28da4d0
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/socfpga/timer.c
> @@ -0,0 +1,149 @@
> +/*
> + * Copyright (C) 2012 Altera Corporation <www.altera.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that 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 <common.h>
> +#include <asm/io.h>
> +#include <asm/arch/timer.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static const struct socfpga_timer *timer_base = (void
> *)CONFIG_SYS_TIMERBASE; +
> +/*
> + * Timer initialization
> + */
> +int timer_init(void)
> +{
> + writel(TIMER_LOAD_VAL, &timer_base->load_val);
> + writel(TIMER_LOAD_VAL, &timer_base->curr_val);
> + writel((readl((&timer_base->ctrl)) | 0x3),
I think you should stick to programming in C here, not ((((LISP)))), so try
cutting down on the ((braces)) :-)
btw. what's this 0x3 magic constant ?
> + (&timer_base->ctrl));
> + return 0;
> +}
[...]
> +#include <common.h>
> +#include <asm/arch/reset_manager.h>
> +#include <asm/io.h>
> +
> +#include <netdev.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +void show_boot_progress(int progress)
> +{
> + debug("Boot reached stage %d\n", progress);
> +}
> +
> +static inline void delay(unsigned long loops)
> +{
> + __asm__ volatile ("1:\n"
> + "subs %0, %1, #1\n"
> + "bne 1b" : "=r" (loops) : "0" (loops));
> +}
Am I flat blind or is this not used here?
[...]
All in all, the code is nice.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
