[U-Boot] [PATCH 03/12] aspeed: Add Timer Support

Maxim Sloyko maxims at google.com
Tue Jan 17 18:57:38 CET 2017


Hi Simon,

This is older series, that I merged into a smaller number of patches and
already addressed all of Tom's comments:
http://lists.denx.de/pipermail/u-boot/2017-January/277883.html

Most of your comments still apply, just want to make sure we are on the
same page.

On Sat, Jan 14, 2017 at 9:13 AM, Simon Glass <sjg at chromium.org> wrote:

> Hi Maxim,
>
> On 4 January 2017 at 12:46, Maxim Sloyko <maxims at google.com> wrote:
> > Add support for timer for Aspeed ast2400/ast2500 devices.
> > The driver actually controls several devices, but because all devices
> > share the same Control Register, it is somewhat difficult to completely
> > decouple them. Since only one timer is needed at the moment, this should
> > be OK.
> >
> > The timer uses fixed clock, so does not rely on a clock driver.
> >
> > Signed-off-by: Maxim Sloyko <maxims at google.com>
> > ---
> >
> >  arch/arm/include/asm/arch-aspeed/timer.h | 54 ++++++++++++++++++
> >  drivers/timer/Kconfig                    |  7 +++
> >  drivers/timer/Makefile                   |  1 +
> >  drivers/timer/ast_timer.c                | 96
> ++++++++++++++++++++++++++++++++
> >  4 files changed, 158 insertions(+)
> >  create mode 100644 arch/arm/include/asm/arch-aspeed/timer.h
> >  create mode 100644 drivers/timer/ast_timer.c
>
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> nits below.
>
> >
> > diff --git a/arch/arm/include/asm/arch-aspeed/timer.h
> b/arch/arm/include/asm/arch-aspeed/timer.h
> > new file mode 100644
> > index 0000000000..87c5b354ec
> > --- /dev/null
> > +++ b/arch/arm/include/asm/arch-aspeed/timer.h
> > @@ -0,0 +1,54 @@
> > +/*
> > + * Copyright (c) 2016 Google, Inc
> > + *
> > + * SPDX-License-Identifier:    GPL-2.0+
> > + */
> > +#ifndef _ASM_ARCH_TIMER_H
> > +#define _ASM_ARCH_TIMER_H
> > +
> > +/* Each timer has 4 control bits in ctrl1 register.
> > + * Timer1 uses bits 0:3, Timer2 uses bits 4:7 and so on,
> > + * such that timer X uses bits (4 * X - 4):(4 * X - 1)
> > + * If the timer does not support PWM, bit 4 is reserved.
> > + */
> > +#define AST_TMC_EN                     (1 << 0)
> > +#define AST_TMC_1MHZ                   (1 << 1)
> > +#define AST_TMC_OVFINTR                        (1 << 2)
> > +#define AST_TMC_PWM                    (1 << 3)
> > +
> > +/* Timers are counted from 1 in the datasheet. */
> > +#define AST_TMC_CTRL1_SHIFT(n)                 (4 * ((n) - 1))
> > +
> > +#define AST_TMC_RATE  (1000*1000)
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +/*
> > + * All timers share control registers, which makes it harder to make
> them
> > + * separate devices. Since only one timer is needed at the moment,
> making
> > + * it this just one device.
> > + */
> > +
> > +struct ast_timer_counter {
> > +       u32 status;
> > +       u32 reload_val;
> > +       u32 match1;
> > +       u32 match2;
> > +};
> > +
> > +struct ast_timer {
> > +       struct ast_timer_counter timers1[3];
> > +       u32 ctrl1;
> > +       u32 ctrl2;
> > +#ifdef CONFIG_ASPEED_AST2500
> > +       u32 ctrl3;
> > +       u32 ctrl1_clr;
> > +#else
> > +       u32 reserved[2];
> > +#endif
> > +       struct ast_timer_counter timers2[5];
> > +};
> > +
> > +#endif  /* __ASSEMBLY__ */
> > +
> > +#endif  /* _ASM_ARCH_TIMER_H */
> > diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
> > index cb18f12fc9..9c5f98bb88 100644
> > --- a/drivers/timer/Kconfig
> > +++ b/drivers/timer/Kconfig
> > @@ -46,4 +46,11 @@ config OMAP_TIMER
> >         help
> >           Select this to enable an timer for Omap devices.
> >
> > +config AST_TIMER
> > +       bool "Aspeed ast2400/ast2500 timer support"
> > +       depends on TIMER
> > +       default y if ARCH_ASPEED
> > +       help
> > +         Select this to enable timer for Aspeed ast2400/ast2500 devices.
>
> Can you add more detail? How many channels? What features are
> supported by the driver?
>
> > +
> >  endmenu
> > diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile
> > index f351fbb4e0..a4b1a486b0 100644
> > --- a/drivers/timer/Makefile
> > +++ b/drivers/timer/Makefile
> > @@ -9,3 +9,4 @@ obj-$(CONFIG_ALTERA_TIMER)      += altera_timer.o
> >  obj-$(CONFIG_SANDBOX_TIMER)    += sandbox_timer.o
> >  obj-$(CONFIG_X86_TSC_TIMER)    += tsc_timer.o
> >  obj-$(CONFIG_OMAP_TIMER)       += omap-timer.o
> > +obj-$(CONFIG_AST_TIMER)        += ast_timer.o
> > diff --git a/drivers/timer/ast_timer.c b/drivers/timer/ast_timer.c
> > new file mode 100644
> > index 0000000000..f644882f40
> > --- /dev/null
> > +++ b/drivers/timer/ast_timer.c
> > @@ -0,0 +1,96 @@
> > +/*
> > + * Copyright 2016 Google Inc.
> > + *
> > + * SPDX-License-Identifier:    GPL-2.0+
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <errno.h>
> > +#include <timer.h>
> > +#include <asm/io.h>
> > +#include <asm/arch/timer.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +#define AST_TICK_TIMER  1
> > +#define AST_TMC_RELOAD_VAL  0xffffffff
> > +
> > +struct ast_timer_priv {
> > +       struct ast_timer *regs;
> > +};
> > +
> > +static struct ast_timer_counter *ast_get_timer_counter(struct ast_timer
> *timer,
> > +                                                      int n)
> > +{
> > +       if (n > 3)
> > +               return &timer->timers2[n - 4];
> > +       else
> > +               return &timer->timers1[n - 1];
> > +}
> > +
> > +static int ast_timer_probe(struct udevice *dev)
> > +{
> > +       struct ast_timer_priv *priv = dev_get_priv(dev);
> > +       struct ast_timer_counter *tmc = ast_get_timer_counter(priv->
> regs,
> > +
>  AST_TICK_TIMER);
>
> I suppose tmc could be in your struct ast_timer_priv to save you doing
> this each time?
>
> > +       struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> > +
> > +       writel(AST_TMC_RELOAD_VAL, &tmc->reload_val);
> > +
> > +       /*
> > +        * Stop the timer. This will also load reload_val into
> > +        * the status register.
> > +        */
> > +       clrbits_le32(&priv->regs->ctrl1,
> > +                    AST_TMC_EN << AST_TMC_CTRL1_SHIFT(AST_TICK_TIMER));
> > +       /* Start the timer from the fixed 1MHz clock. */
> > +       setbits_le32(&priv->regs->ctrl1,
> > +                    (AST_TMC_EN | AST_TMC_1MHZ) <<
> > +                    AST_TMC_CTRL1_SHIFT(AST_TICK_TIMER));
> > +
> > +       uc_priv->clock_rate = AST_TMC_RATE;
> > +
> > +       return 0;
> > +}
> > +
> > +static int ast_timer_get_count(struct udevice *dev, u64 *count)
> > +{
> > +       struct ast_timer_priv *priv = dev_get_priv(dev);
> > +       struct ast_timer_counter *tmc = ast_get_timer_counter(priv->
> regs,
> > +
>  AST_TICK_TIMER);
> > +
> > +       *count = AST_TMC_RELOAD_VAL - readl(&tmc->status);
> > +
> > +       return 0;
> > +}
> > +
> > +static int ast_timer_ofdata_to_platdata(struct udevice *dev)
> > +{
> > +       struct ast_timer_priv *priv = dev_get_priv(dev);
> > +
> > +       priv->regs = (struct ast_timer *)dev_get_addr(dev);
>
> You can use dev_get_addr_ptr() if you like.
>
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct timer_ops ast_timer_ops = {
> > +       .get_count = ast_timer_get_count,
> > +};
> > +
> > +static const struct udevice_id ast_timer_ids[] = {
> > +       { .compatible = "aspeed,ast2500-timer" },
> > +       { .compatible = "aspeed,ast2400-timer" },
> > +       { }
> > +};
> > +
> > +U_BOOT_DRIVER(sandbox_timer) = {
>
> s/sandbox/ast/ or something, as Tom, mentioned.
>
> > +       .name = "ast_timer",
> > +       .id = UCLASS_TIMER,
> > +       .of_match = ast_timer_ids,
> > +       .probe = ast_timer_probe,
> > +       .priv_auto_alloc_size = sizeof(struct ast_timer_priv),
> > +       .ofdata_to_platdata = ast_timer_ofdata_to_platdata,
> > +       .ops = &ast_timer_ops,
> > +       .flags = DM_FLAG_PRE_RELOC,
> > +};
> > --
> > 2.11.0.390.gc69c2f50cf-goog
>
> Regards,
> Simon
>
> >
>



-- 
*M*axim *S*loyko


More information about the U-Boot mailing list