[U-Boot] [PATCH 3/4] arm: Support new Xilinx Zynq platform
Michal Simek
monstr at monstr.eu
Tue Aug 14 20:39:32 CEST 2012
On 08/14/2012 08:19 PM, Joe Hershberger wrote:
> Hi Michal,
>
> On Tue, Aug 14, 2012 at 12:39 PM, Michal Simek <monstr at monstr.eu> wrote:
>> On 08/14/2012 07:15 PM, Joe Hershberger wrote:
>>>
>>> Hi Michal,
>>>
>>> On Tue, Aug 14, 2012 at 12:11 PM, Michal Simek <monstr at 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 at 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 at monstr.eu>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Add timer driver.
>>>>>>>>
>>>>>>>> Signed-off-by: Michal Simek <monstr at 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 at 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 at monstr.eu>
>>>>>>>> + * Copyright (C) 2011-2012 Xilinx, Inc. All rights reserved.
>>>>>>>> + *
>>>>>>>> + * (C) Copyright 2008
>>>>>>>> + * Guennadi Liakhovetki, DENX Software Engineering, <lg at denx.de>
>>>>>>>> + *
>>>>>>>> + * (C) Copyright 2004
>>>>>>>> + * Philippe Robin, ARM Ltd. <philippe.robin at arm.com>
>>>>>>>> + *
>>>>>>>> + * (C) Copyright 2002-2004
>>>>>>>> + * Gary Jennejohn, DENX Software Engineering, <gj at denx.de>
>>>>>>>> + *
>>>>>>>> + * (C) Copyright 2003
>>>>>>>> + * Texas Instruments <www.ti.com>
>>>>>>>> + *
>>>>>>>> + * (C) Copyright 2002
>>>>>>>> + * Sysgo Real-Time Solutions, GmbH <www.elinos.com>
>>>>>>>> + * Marius Groeger <mgroeger at sysgo.de>
>>>>>>>> + *
>>>>>>>> + * (C) Copyright 2002
>>>>>>>> + * Sysgo Real-Time Solutions, GmbH <www.elinos.com>
>>>>>>>> + * Alex Zuepke <azu at 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.
>>
>>
>> Yes, that's partially clear case if Microblaze doesn't want to use it.
>>
>> Zynq can also use axi_timer. We have this driver in Microblaze folder.
>
> This would be bad because it would assume some timer in the fabric.
>
>> This is driver sharing across architectures. IRC there was any discussion
>> about
>> moving drivers to generic location in past but this has never happen. (BTW:
>> this
>> driver can be used by xilinx ppc405 and ppc440)
>
> I guess by "this driver" you mean axi_timer and not scu_timer.
yep
>
>>
>>
>> If Albert tends to move this driver to any general location I have no
>> problem to do
>> in spite of there is probably not any other armv7 close which will use it.
>>
>> From my point of view if there is not other armv7 clone which will use it,
>> make sense to keep this in zynq folder. And move it to generic location
>> when any other armv7 wants to use.
>
> This is exactly the opposite of what we should be doing. It is true
> that this refactoring has not been very wide-spread yet, but certainly
> for new things, the idea is to only put things into a less generic
> place if there is no reasonable chance that it will be reused outside
> of the more specific scope. If it is available in other places in
> hardware, make it generic. It's not as good to expect that every new
> board should have to search every other board dir in the hope that he
> finds a driver that he can reuse. He should be able to look in the
> arch (or even drivers/ if applicable) and complete the search sooner.
> Why force extra work to move it later. To the best of your ability,
> put it in the best place. If this driver is available for all cortex
> a9 based SoCs, then chances are every new one added from now on will
> use this instead of writing a new one, even if there are other timers
> available.
I agree with that. Have no problem to move this driver to arch/arm/cpu/armv7/
folder or even to drivers/timer if is what we should do.
I am just not sure if this should be done before new DM model.
Marek: I believe you are going to change all timer drivers. Are you going to
move all of them to generic location?
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
More information about the U-Boot
mailing list