From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Simek Date: Tue, 14 Aug 2012 20:39:32 +0200 Subject: [U-Boot] [PATCH 3/4] arm: Support new Xilinx Zynq platform In-Reply-To: References: <1344944567-1694-1-git-send-email-monstr@monstr.eu> <1344944567-1694-4-git-send-email-monstr@monstr.eu> <502A7C99.40303@monstr.eu> <502A86C8.1040508@monstr.eu> <502A8D5A.1020505@monstr.eu> Message-ID: <502A9B64.3060007@monstr.eu> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 08/14/2012 08:19 PM, Joe Hershberger wrote: > Hi Michal, > > On Tue, Aug 14, 2012 at 12:39 PM, Michal Simek wrote: >> On 08/14/2012 07:15 PM, Joe Hershberger wrote: >>> >>> Hi Michal, >>> >>> On Tue, Aug 14, 2012 at 12:11 PM, Michal Simek wrote: >>>> >>>> On 08/14/2012 06:41 PM, Joe Hershberger wrote: >>>>> >>>>> >>>>> Hi Michal, >>>>> >>>>> On Tue, Aug 14, 2012 at 11:28 AM, Michal Simek wrote: >>>>>> >>>>>> >>>>>> On 08/14/2012 05:36 PM, Joe Hershberger wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> Hi Michal, >>>>>>> >>>>>>> On Tue, Aug 14, 2012 at 6:42 AM, Michal Simek >>>>>>> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Add timer driver. >>>>>>>> >>>>>>>> Signed-off-by: Michal Simek >>>>>>>> --- >>>>>>>> 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, >>>>>>>> +# >>>>>>>> +# 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 >>>>>>>> + * Copyright (C) 2011-2012 Xilinx, Inc. All rights reserved. >>>>>>>> + * >>>>>>>> + * (C) Copyright 2008 >>>>>>>> + * Guennadi Liakhovetki, DENX Software Engineering, >>>>>>>> + * >>>>>>>> + * (C) Copyright 2004 >>>>>>>> + * Philippe Robin, ARM Ltd. >>>>>>>> + * >>>>>>>> + * (C) Copyright 2002-2004 >>>>>>>> + * Gary Jennejohn, DENX Software Engineering, >>>>>>>> + * >>>>>>>> + * (C) Copyright 2003 >>>>>>>> + * Texas Instruments >>>>>>>> + * >>>>>>>> + * (C) Copyright 2002 >>>>>>>> + * Sysgo Real-Time Solutions, GmbH >>>>>>>> + * Marius Groeger >>>>>>>> + * >>>>>>>> + * (C) Copyright 2002 >>>>>>>> + * Sysgo Real-Time Solutions, GmbH >>>>>>>> + * Alex Zuepke >>>>>>>> + * >>>>>>>> + * 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 >>>>>>>> +#include >>>>>>>> +#include >>>>>>>> + >>>>>>>> +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