diff mbox

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

Message ID 1344944567-1694-4-git-send-email-monstr@monstr.eu
State Superseded
Headers show

Commit Message

Michal Simek Aug. 14, 2012, 11:42 a.m. UTC
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

Comments

Joe Hershberger Aug. 14, 2012, 3:36 p.m. UTC | #1
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@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.

> +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.

> +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.

> +
> +static struct scu_timer *timer_base = CONFIG_XPSS_SCUTIMER_BASEADDR;
> +
> +#define XSCUTIMER_CONTROL_PRESCALER_MASK       0x0000FF00 /* Prescaler */
> +#define XSCUTIMER_CONTROL_PRESCALER_SHIFT      8
> +#define XSCUTIMER_CONTROL_AUTO_RELOAD_MASK     0x00000002 /* Auto-reload */
> +#define XSCUTIMER_CONTROL_ENABLE_MASK          0x00000001 /* Timer enable */
> +
> +#define TIMER_LOAD_VAL 0xFFFFFFFF
> +#define TIMER_PRESCALE 255
> +#define TIMER_TICK_HZ  (CONFIG_CPU_FREQ_HZ / 2 / TIMER_PRESCALE)
> +
> +int timer_init(void)
> +{
> +       u32 val;
> +
> +       /* Load the timer counter register */
> +       writel(0xFFFFFFFF, &timer_base->counter);
> +
> +       /* Start the A9Timer device */
> +       val = readl(&timer_base->control);
> +       /* Enable Auto reload mode */
> +       val |= XSCUTIMER_CONTROL_AUTO_RELOAD_MASK;
> +       /* Clear prescaler control bits */
> +       val &= ~XSCUTIMER_CONTROL_PRESCALER_MASK;
> +       /* Set prescaler value */
> +       val |= (TIMER_PRESCALE << XSCUTIMER_CONTROL_PRESCALER_SHIFT);
> +       /* Enable the decrementer */
> +       val |= XSCUTIMER_CONTROL_ENABLE_MASK;
> +       writel(val, &timer_base->control);
> +
> +       /* Reset time */
> +       gd->lastinc = readl(&timer_base->counter) /
> +                                       (TIMER_TICK_HZ / CONFIG_SYS_HZ);
> +       gd->tbl = 0;
> +
> +       return 0;
> +}
> +
> +/*
> + * This function is derived from PowerPC code (read timebase as long long).
> + * On ARM it just returns the timer value.
> + */
> +ulong get_timer_masked(void)
> +{
> +       ulong now;
> +
> +       now = readl(&timer_base->counter) / (TIMER_TICK_HZ / CONFIG_SYS_HZ);
> +
> +       if (gd->lastinc >= now) {
> +               /* Normal mode */
> +               gd->tbl += gd->lastinc - now;
> +       } else {
> +               /* We have an overflow ... */
> +               gd->tbl += gd->lastinc + TIMER_LOAD_VAL - now;
> +       }
> +       gd->lastinc = now;
> +
> +       return gd->tbl;
> +}
> +
> +void __udelay(unsigned long usec)
> +{
> +       unsigned long long tmp;
> +       ulong tmo;
> +
> +       tmo = usec / (1000000 / CONFIG_SYS_HZ);
> +       tmp = get_ticks() + tmo; /* Get current timestamp */
> +
> +       while (get_ticks() < tmp) { /* Loop till event */
> +                /* NOP */;
> +       }
> +}
> +
> +/* Timer without interrupts */
> +ulong get_timer(ulong base)
> +{
> +       return get_timer_masked() - base;
> +}
> +
> +/*
> + * This function is derived from PowerPC code (read timebase as long long).
> + * On ARM it just returns the timer value.
> + */
> +unsigned long long get_ticks(void)
> +{
> +       return get_timer(0);
> +}
> +
> +/*
> + * This function is derived from PowerPC code (timebase clock frequency).
> + * On ARM it returns the number of timer ticks per second.
> + */
> +ulong get_tbclk(void)
> +{
> +       return CONFIG_SYS_HZ;
> +}
> --
> 1.7.0.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Michal Simek Aug. 14, 2012, 4:28 p.m. UTC | #2
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@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.

Thanks,
Michal
Joe Hershberger Aug. 14, 2012, 4:41 p.m. UTC | #3
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@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/.

-Joe
Michal Simek Aug. 14, 2012, 5:11 p.m. UTC | #4
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@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.

Any input from arm custodian will be helpful.

Thanks,
Michal
Joe Hershberger Aug. 14, 2012, 5:15 p.m. UTC | #5
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@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
Michal Simek Aug. 14, 2012, 5:39 p.m. UTC | #6
On 08/14/2012 07:15 PM, Joe Hershberger wrote:
> 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@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.

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 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)


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.

Thanks,
Michal
Joe Hershberger Aug. 14, 2012, 6:19 p.m. UTC | #7
Hi Michal,

On Tue, Aug 14, 2012 at 12:39 PM, Michal Simek <monstr@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@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@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.
>
>
> 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.

>
>
> 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.

Thanks,
-Joe
Michal Simek Aug. 14, 2012, 6:39 p.m. UTC | #8
On 08/14/2012 08:19 PM, Joe Hershberger wrote:
> Hi Michal,
>
> On Tue, Aug 14, 2012 at 12:39 PM, Michal Simek <monstr@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@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@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.
>>
>>
>> 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
Marek Vasut Aug. 14, 2012, 8:18 p.m. UTC | #9
Dear Michal Simek,

> On 08/14/2012 08:19 PM, Joe Hershberger wrote:
> > Hi Michal,
[...]
CUT!

> Marek: I believe you are going to change all timer drivers. Are you going
> to move all of them to generic location?

I'm not moving timer drivers in the first wave, so hacking on them should be 
safe sail so far.

> Thanks,
> Michal

Best regards,
Marek Vasut
diff mbox

Patch

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@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
+
+COBJS	= timer.o
+
+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 */
+};
+
+static struct scu_timer *timer_base = CONFIG_XPSS_SCUTIMER_BASEADDR;
+
+#define XSCUTIMER_CONTROL_PRESCALER_MASK	0x0000FF00 /* Prescaler */
+#define XSCUTIMER_CONTROL_PRESCALER_SHIFT	8
+#define XSCUTIMER_CONTROL_AUTO_RELOAD_MASK	0x00000002 /* Auto-reload */
+#define XSCUTIMER_CONTROL_ENABLE_MASK		0x00000001 /* Timer enable */
+
+#define TIMER_LOAD_VAL 0xFFFFFFFF
+#define TIMER_PRESCALE 255
+#define TIMER_TICK_HZ  (CONFIG_CPU_FREQ_HZ / 2 / TIMER_PRESCALE)
+
+int timer_init(void)
+{
+	u32 val;
+
+	/* Load the timer counter register */
+	writel(0xFFFFFFFF, &timer_base->counter);
+
+	/* Start the A9Timer device */
+	val = readl(&timer_base->control);
+	/* Enable Auto reload mode */
+	val |= XSCUTIMER_CONTROL_AUTO_RELOAD_MASK;
+	/* Clear prescaler control bits */
+	val &= ~XSCUTIMER_CONTROL_PRESCALER_MASK;
+	/* Set prescaler value */
+	val |= (TIMER_PRESCALE << XSCUTIMER_CONTROL_PRESCALER_SHIFT);
+	/* Enable the decrementer */
+	val |= XSCUTIMER_CONTROL_ENABLE_MASK;
+	writel(val, &timer_base->control);
+
+	/* Reset time */
+	gd->lastinc = readl(&timer_base->counter) /
+					(TIMER_TICK_HZ / CONFIG_SYS_HZ);
+	gd->tbl = 0;
+
+	return 0;
+}
+
+/*
+ * This function is derived from PowerPC code (read timebase as long long).
+ * On ARM it just returns the timer value.
+ */
+ulong get_timer_masked(void)
+{
+	ulong now;
+
+	now = readl(&timer_base->counter) / (TIMER_TICK_HZ / CONFIG_SYS_HZ);
+
+	if (gd->lastinc >= now) {
+		/* Normal mode */
+		gd->tbl += gd->lastinc - now;
+	} else {
+		/* We have an overflow ... */
+		gd->tbl += gd->lastinc + TIMER_LOAD_VAL - now;
+	}
+	gd->lastinc = now;
+
+	return gd->tbl;
+}
+
+void __udelay(unsigned long usec)
+{
+	unsigned long long tmp;
+	ulong tmo;
+
+	tmo = usec / (1000000 / CONFIG_SYS_HZ);
+	tmp = get_ticks() + tmo; /* Get current timestamp */
+
+	while (get_ticks() < tmp) { /* Loop till event */
+		 /* NOP */;
+	}
+}
+
+/* Timer without interrupts */
+ulong get_timer(ulong base)
+{
+	return get_timer_masked() - base;
+}
+
+/*
+ * This function is derived from PowerPC code (read timebase as long long).
+ * On ARM it just returns the timer value.
+ */
+unsigned long long get_ticks(void)
+{
+	return get_timer(0);
+}
+
+/*
+ * This function is derived from PowerPC code (timebase clock frequency).
+ * On ARM it returns the number of timer ticks per second.
+ */
+ulong get_tbclk(void)
+{
+	return CONFIG_SYS_HZ;
+}