Hi Thomas,
On 4 October 2015 at 14:56, Thomas Chou <tho...@wytron.com.tw> wrote:
> Implement a Timer uclass to work with lib/time.c.
>
> Signed-off-by: Thomas Chou <tho...@wytron.com.tw>
> Reviewed-by: Simon Glass <s...@chromium.org>
Sorry, after a lot of consideration I'd like to retract that :-(
Please see below.
> ---
> 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.
> v4
> add comment about timer hardware.
>
> common/board_r.c | 3 +++
> drivers/Kconfig | 2 ++
> drivers/Makefile | 1 +
> drivers/timer/Kconfig | 12 +++++++++
> drivers/timer/Makefile | 7 +++++
> drivers/timer/timer-uclass.c | 44 +++++++++++++++++++++++++++++++
> include/asm-generic/global_data.h | 3 +++
> include/dm/uclass-id.h | 1 +
> include/timer.h | 52 +++++++++++++++++++++++++++++++++++++
> lib/time.c | 54
> +++++++++++++++++++++++++++++++++++++++
> 10 files changed, 179 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
Can you please split this into a patch that adds the uclass and one
that plumbs it into board_r.c?
Also can you add a patch with a sandbox timer driver and a test (in
test/dm) for the timer?
>
> 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..54a4c40
> --- /dev/null
> +++ b/drivers/timer/Kconfig
> @@ -0,0 +1,12 @@
> +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. The timer is usually a 32 bits free-running up
> + counter. There may be no real tick, and no timer interrupt.
> +
> +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 <tho...@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..bcf1dde
> --- /dev/null
> +++ b/drivers/timer/timer-uclass.c
> @@ -0,0 +1,44 @@
> +/*
> + * Copyright (C) 2015 Thomas Chou <tho...@wytron.com.tw>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <timer.h>
> +
> +/*
> + * Implement a Timer uclass to work with lib/time.c. The timer is usually
> + * a 32 bits free-running up counter. The get_clock() method is used to get
> + * the input clock frequency of the timer. The get_count() method is used
> + * get the current 32 bits count value. If the hardware is counting down,
> + * the value should be inversed inside the method. There may be no real
> + * tick, and no timer interrupt.
> + */
> +
> +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);
> +}
> +
> +int timer_get_clock(struct udevice *dev, unsigned long *freq)
Probably timer_get_rate() is better - we yes it has a clock but it is
the clock rate that we are returning.
Isn't the frequency always the same for a given timer? I think this
should be stored in the uclass private data. The timer could do this
in its probe function. Then this function can just return that value
rather than needing a driver method:
unsigned long timer_get_rate(struct udevice *dev)
{
struct uc_priv *priv = dev_get_uclass_pric(dev);
return priv->clock_rate;
}
> +{
> + const struct dm_timer_ops *ops = device_get_ops(dev);
> +
> + if (!ops->get_clock)
> + return -ENOSYS;
> +
> + return ops->get_clock(dev, freq);
> +}
> +
> +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 <tho...@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..ba063cf 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 notrace dm_timer_init(void)
> +{
> + 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
