[U-Boot] [PATCH v3 1/2] dm: implement a Timer uclass

Simon Glass sjg at chromium.org
Sat Oct 3 16:29:32 CEST 2015


Hi Thomas,

On 3 October 2015 at 10:19, Thomas Chou <thomas at wytron.com.tw> wrote:
> Implement a Timer uclass to work with lib/time.c.
>
> Signed-off-by: Thomas Chou <thomas at wytron.com.tw>
> ---
> v2
>   fix coding style.
> v3
>   add description to Kconfig as Simon suggested.
>   move timer.c code to lib/time.c.
>   add dm_timer dev to global data.
>   remove timer_init().
>   change API name get_clock.
>
>  common/board_r.c                  |  3 +++
>  drivers/Kconfig                   |  2 ++
>  drivers/Makefile                  |  1 +
>  drivers/timer/Kconfig             | 11 ++++++++
>  drivers/timer/Makefile            |  7 +++++
>  drivers/timer/timer-uclass.c      | 35 +++++++++++++++++++++++++
>  include/asm-generic/global_data.h |  3 +++
>  include/dm/uclass-id.h            |  1 +
>  include/timer.h                   | 52 +++++++++++++++++++++++++++++++++++++
>  lib/time.c                        | 54 +++++++++++++++++++++++++++++++++++++++
>  10 files changed, 169 insertions(+)
>  create mode 100644 drivers/timer/Kconfig
>  create mode 100644 drivers/timer/Makefile
>  create mode 100644 drivers/timer/timer-uclass.c
>  create mode 100644 include/timer.h

Reviewed-by: Simon Glass <sjg at chromium.org>

Please see one nit below.

>
> diff --git a/common/board_r.c b/common/board_r.c
> index f8c1baa..aaf390e 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -290,6 +290,9 @@ static int initr_dm(void)
>         /* Save the pre-reloc driver model and start a new one */
>         gd->dm_root_f = gd->dm_root;
>         gd->dm_root = NULL;
> +#ifdef CONFIG_DM_TIMER
> +       gd->dm_timer = NULL;
> +#endif
>         return dm_init_and_scan(false);
>  }
>  #endif
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 63c92c5..f9496f7 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -56,6 +56,8 @@ source "drivers/spi/Kconfig"
>
>  source "drivers/thermal/Kconfig"
>
> +source "drivers/timer/Kconfig"
> +
>  source "drivers/tpm/Kconfig"
>
>  source "drivers/usb/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 9d0a595..692da78 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -48,6 +48,7 @@ obj-y += pcmcia/
>  obj-y += dfu/
>  obj-y += rtc/
>  obj-y += sound/
> +obj-y += timer/
>  obj-y += tpm/
>  obj-y += twserial/
>  obj-y += video/
> diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
> new file mode 100644
> index 0000000..5228e26
> --- /dev/null
> +++ b/drivers/timer/Kconfig
> @@ -0,0 +1,11 @@
> +menu "Timer Support"
> +
> +config DM_TIMER
> +       bool "Enable Driver Model for Timer drivers"
> +       depends on DM
> +       help
> +         Enable driver model for Timer access. It uses the same API as
> +         lib/time.c. But now implemented by the uclass. The first timer
> +         will be used.
> +
> +endmenu
> diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile
> new file mode 100644
> index 0000000..a24179a
> --- /dev/null
> +++ b/drivers/timer/Makefile
> @@ -0,0 +1,7 @@
> +#
> +# Copyright (C) 2015 Thomas Chou <thomas at wytron.com.tw>
> +#
> +# SPDX-License-Identifier:     GPL-2.0+
> +#
> +
> +obj-$(CONFIG_DM_TIMER)         += timer-uclass.o
> diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
> new file mode 100644
> index 0000000..8071fcc
> --- /dev/null
> +++ b/drivers/timer/timer-uclass.c
> @@ -0,0 +1,35 @@
> +/*
> + * Copyright (C) 2015 Thomas Chou <thomas at wytron.com.tw>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <timer.h>
> +
> +int timer_get_count(struct udevice *dev, unsigned long *count)
> +{
> +       const struct dm_timer_ops *ops = device_get_ops(dev);
> +
> +       if (!ops->get_count)
> +               return -ENOSYS;
> +
> +       return ops->get_count(dev, count);

I'm not too keen on get_count(). Isn't this required to be in
milliiseconds now? If so, how about get_ms()?

> +}
> +
> +int timer_get_clock(struct udevice *dev, unsigned long *freq)
> +{
> +       const struct dm_timer_ops *ops = device_get_ops(dev);
> +
> +       if (!ops->get_clock)
> +               return -ENOSYS;
> +
> +       return ops->get_clock(dev, freq);

Isn't this always 1000?

> +}
> +
> +UCLASS_DRIVER(timer) = {
> +       .id             = UCLASS_TIMER,
> +       .name           = "timer",
> +};
> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
> index 2155265..ebecb5f 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -69,6 +69,9 @@ typedef struct global_data {
>         struct udevice  *dm_root_f;     /* Pre-relocation root instance */
>         struct list_head uclass_root;   /* Head of core tree */
>  #endif
> +#ifdef CONFIG_DM_TIMER
> +       struct udevice  *dm_timer;      /* Timer instance for Driver Model */
> +#endif
>
>         const void *fdt_blob;   /* Our device tree, NULL if none */
>         void *new_fdt;          /* Relocated FDT */
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 1eeec74..aff34a4 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -56,6 +56,7 @@ enum uclass_id {
>         UCLASS_SPI_GENERIC,     /* Generic SPI flash target */
>         UCLASS_SYSCON,          /* System configuration device */
>         UCLASS_THERMAL,         /* Thermal sensor */
> +       UCLASS_TIMER,           /* Timer device */
>         UCLASS_TPM,             /* Trusted Platform Module TIS interface */
>         UCLASS_USB,             /* USB bus */
>         UCLASS_USB_DEV_GENERIC, /* USB generic device */
> diff --git a/include/timer.h b/include/timer.h
> new file mode 100644
> index 0000000..c18a195
> --- /dev/null
> +++ b/include/timer.h
> @@ -0,0 +1,52 @@
> +/*
> + * Copyright (C) 2015 Thomas Chou <thomas at wytron.com.tw>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef _DM_TIMER_H_
> +#define _DM_TIMER_H_
> +
> +/*
> + * Get the current timer count
> + *
> + * @dev: The Timer device
> + * @count: pointer that returns the current timer count
> + * @return: 0 if OK, -ve on error
> + */
> +int timer_get_count(struct udevice *dev, unsigned long *count);
> +/*
> + * Get the timer input clock frequency
> + *
> + * @dev: The Timer device
> + * @freq: pointer that returns the timer clock frequency
> + * @return: 0 if OK, -ve on error
> + */
> +int timer_get_clock(struct udevice *dev, unsigned long *freq);
> +
> +/*
> + * struct dm_timer_ops - Driver model Timer operations
> + *
> + * The uclass interface is implemented by all Timer devices which use
> + * driver model.
> + */
> +struct dm_timer_ops {
> +       /*
> +        * Get the current timer count
> +        *
> +        * @dev: The Timer device
> +        * @count: pointer that returns the current timer count
> +        * @return: 0 if OK, -ve on error
> +        */
> +       int (*get_count)(struct udevice *dev, unsigned long *count);
> +       /*
> +        * Get the timer input clock frequency
> +        *
> +        * @dev: The Timer device
> +        * @freq: pointer that returns the timer clock frequency
> +        * @return: 0 if OK, -ve on error
> +        */
> +       int (*get_clock)(struct udevice *dev, unsigned long *freq);
> +};
> +
> +#endif /* _DM_TIMER_H_ */
> diff --git a/lib/time.c b/lib/time.c
> index 477440d..6b7342e 100644
> --- a/lib/time.c
> +++ b/lib/time.c
> @@ -6,6 +6,9 @@
>   */
>
>  #include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <timer.h>
>  #include <watchdog.h>
>  #include <div64.h>
>  #include <asm/io.h>
> @@ -37,6 +40,57 @@ unsigned long notrace timer_read_counter(void)
>  extern unsigned long __weak timer_read_counter(void);
>  #endif
>
> +#ifdef CONFIG_DM_TIMER
> +static int dm_timer_init(void)

nit: need notrace on this one too.

> +{
> +       struct udevice *dev;
> +       int ret;
> +
> +       if (!gd->dm_timer) {
> +               ret = uclass_first_device(UCLASS_TIMER, &dev);
> +               if (ret)
> +                       return ret;
> +               if (!dev)
> +                       return -ENODEV;
> +               gd->dm_timer = dev;
> +       }
> +
> +       return 0;
> +}
> +
> +ulong notrace get_tbclk(void)
> +{
> +       unsigned long freq;
> +       int ret;
> +
> +       ret = dm_timer_init();
> +       if (ret)
> +               return ret;
> +
> +       ret = timer_get_clock(gd->dm_timer, &freq);
> +       if (ret)
> +               return ret;
> +
> +       return freq;
> +}
> +
> +unsigned long notrace timer_read_counter(void)
> +{
> +       unsigned long count;
> +       int ret;
> +
> +       ret = dm_timer_init();
> +       if (ret)
> +               return ret;
> +
> +       ret = timer_get_count(gd->dm_timer, &count);
> +       if (ret)
> +               return ret;
> +
> +       return count;
> +}
> +#endif /* CONFIG_DM_TIMER */
> +
>  uint64_t __weak notrace get_ticks(void)
>  {
>         unsigned long now = timer_read_counter();
> --
> 2.1.4
>

Regards,
Simon


More information about the U-Boot mailing list