[U-Boot] [PATCH 3/4] arm: Support new Xilinx Zynq platform

Joe Hershberger
2012-08-14T17:15:48Z

Hi Michal,

On Tue, Aug 14, 2012 at 12:11 PM, Michal Simek <monstr@monstr.eu> wrote:
> On 08/14/2012 06:41 PM, Joe Hershberger wrote:
>>
>> Hi Michal,
>>
>> On Tue, Aug 14, 2012 at 11:28 AM, Michal Simek <monstr@monstr.eu> wrote:
>>>
>>> On 08/14/2012 05:36 PM, Joe Hershberger wrote:
>>>>
>>>>
>>>> Hi Michal,
>>>>
>>>> On Tue, Aug 14, 2012 at 6:42 AM, Michal Simek <monstr@monstr.eu> wrote:
>>>>>
>>>>>
>>>>> Add timer driver.
>>>>>
>>>>> Signed-off-by: Michal Simek <monstr@monstr.eu>
>>>>> ---
>>>>> arch/arm/cpu/armv7/zynq/Makefile | 48 ++++++++++++
>>>>> arch/arm/cpu/armv7/zynq/timer.c | 151
>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>> 2 files changed, 199 insertions(+), 0 deletions(-)
>>>>> create mode 100644 arch/arm/cpu/armv7/zynq/Makefile
>>>>> create mode 100644 arch/arm/cpu/armv7/zynq/timer.c
>>>>>
>>>>> diff --git a/arch/arm/cpu/armv7/zynq/Makefile
>>>>> b/arch/arm/cpu/armv7/zynq/Makefile
>>>>> new file mode 100644
>>>>> index 0000000..814c1d4
>>>>> --- /dev/null
>>>>> +++ b/arch/arm/cpu/armv7/zynq/Makefile
>>>>> @@ -0,0 +1,48 @@
>>>>> +#
>>>>> +# (C) Copyright 2000-2003
>>>>> +# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>>>>> +#
>>>>> +# (C) Copyright 2008
>>>>> +# Guennadi Liakhovetki, DENX Software Engineering, <lg@denx.de>
>>>>> +#
>>>>> +# 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
>>>>> +
>>>>
>>>>
>>>>
>>>> You should include the lowlevel_init.o here instead of in the board.
>>>
>>>
>>>
>>> Probably make sense.
>>>
>>>
>>>
>>>>
>>>>> +COBJS = timer.o
>>>>> +
>>>>
>>>>
>>>>
>>>> Preferably emulate the Makefile in arch/arm/cpu/armv7/tegra2/. By
>>>> that I mean use COBJS-y instead of COBJS directly. This is more
>>>> forward looking to allow for features to be disabled in the future.
>>>
>>>
>>>
>>> no problem with that.
>>>
>>>
>>>>
>>>>> +SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c)
>>>>> +OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS))
>>>>> +
>>>>> +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/zynq/timer.c
>>>>> b/arch/arm/cpu/armv7/zynq/timer.c
>>>>> new file mode 100644
>>>>> index 0000000..d79da97
>>>>> --- /dev/null
>>>>> +++ b/arch/arm/cpu/armv7/zynq/timer.c
>>>>> @@ -0,0 +1,151 @@
>>>>> +/*
>>>>> + * Copyright (C) 2012 Michal Simek <monstr@monstr.eu>
>>>>> + * Copyright (C) 2011-2012 Xilinx, Inc. All rights reserved.
>>>>> + *
>>>>> + * (C) Copyright 2008
>>>>> + * Guennadi Liakhovetki, DENX Software Engineering, <lg@denx.de>
>>>>> + *
>>>>> + * (C) Copyright 2004
>>>>> + * Philippe Robin, ARM Ltd. <philippe.robin@arm.com>
>>>>> + *
>>>>> + * (C) Copyright 2002-2004
>>>>> + * Gary Jennejohn, DENX Software Engineering, <gj@denx.de>
>>>>> + *
>>>>> + * (C) Copyright 2003
>>>>> + * Texas Instruments <www.ti.com>
>>>>> + *
>>>>> + * (C) Copyright 2002
>>>>> + * Sysgo Real-Time Solutions, GmbH <www.elinos.com>
>>>>> + * Marius Groeger <mgroeger@sysgo.de>
>>>>> + *
>>>>> + * (C) Copyright 2002
>>>>> + * Sysgo Real-Time Solutions, GmbH <www.elinos.com>
>>>>> + * Alex Zuepke <azu@sysgo.de>
>>>>> + *
>>>>> + * 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 <common.h>
>>>>> +#include <div64.h>
>>>>> +#include <asm/io.h>
>>>>> +
>>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>>> +
>>>>> +struct scu_timer {
>>>>> + u32 load; /* Timer Load Register */
>>>>> + u32 counter; /* Timer Counter Register */
>>>>> + u32 control; /* Timer Control Register */
>>>>> +};
>>>>
>>>>
>>>>
>>>> You are using the timer in the ARM Cortex A9 core. This is not
>>>> Zynq-specific in any way. It should be made available in a different
>>>> place. Probably in arch/arm/lib. It should be stripped on any "XSCU"
>>>> references. Use ARM names instead.
>>>
>>>
>>>
>>>
>>> Based on this I can't see the reason why XSCU should be stripped.
>>>
>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0407f/BABDEAGJ.html
>>>
>>> It is SCU private timer. Agree that we can remove X prefix.
>>>
>>>
>>> I don't think that arch/arm/lib is the proper location for that.
>>> I am not arm specialist to say that this timer is available in all arm
>>> families
>>> to be in lib folder.
>>
>>
>> I'm not sure that it is available on more than armv7 (like
>> arch/arm/lib/cache-pl310.c). If it is only available in armv7, then
>> it can go in arch/arm/cpu/armv7/.
>
>
> For me was just safer to use it in zynq folder because I am not familiar
> with others ARMs.
> + I see that other armv7 cpus uses own timer drivers.

That is true, but they seem to be using other timer hardware instead
of the ARM core timers. For instance, if you were using the Zynq's
Triple Timer Counter module instead, then it would make sense to have
this in the zynq directory.

>
> Any input from arm custodian will be helpful.

Albert?

>
>
> Thanks,
> Michal
> --
> Michal Simek, Ing. (M.Eng)
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
> Microblaze U-BOOT custodian