[U-Boot] [RFC PATCH 3/6] rockchip: timer: add device-model timer driver for RK3368 (and similar)

[Simon Glass]

On 28 July 2017 at 10:31, Philipp Tomsich
<philipp.tomsich at theobroma-systems.com> wrote:
> This adds a device-model driver for the timer block in the RK3368 (and
> similar devices that share the same timer block, such as the RK3288) for
> the down-counting (i.e. non-secure) timers.
>
> This allows us to configure U-Boot for the RK3368 in such a way that
> we can run with the secure timer inaccessible or uninitialised (note
> that the ARMv8 generic timer does not count, if the secure timer is
> not enabled).
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
> ---
>
>  drivers/timer/Kconfig          |   7 +++
>  drivers/timer/Makefile         |   1 +
>  drivers/timer/rockchip_timer.c | 105 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 113 insertions(+)
>  create mode 100644 drivers/timer/rockchip_timer.c

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

See a few comments below.

>
> diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
> index fb5af4d..5dea404 100644
> --- a/drivers/timer/Kconfig
> +++ b/drivers/timer/Kconfig
> @@ -104,4 +104,11 @@ config AE3XX_TIMER
>         help
>           Select this to enable a timer for AE3XX devices.
>
> +config ROCKCHIP_TIMER
> +        bool "Rockchip timer support"
> +       depends on TIMER
> +       help
> +         Select this to enable support for the timer block found on

I'm not so keen on the word 'block'. Maybe it should be 'IP block'? Or
perhaps don't mention block at all and say that this is the timer.

> +         Rockchip devices.
> +
>  endmenu
> diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile
> index d16ea53..fa7ce7c 100644
> --- a/drivers/timer/Makefile
> +++ b/drivers/timer/Makefile
> @@ -14,3 +14,4 @@ obj-$(CONFIG_STI_TIMER)               += sti-timer.o
>  obj-$(CONFIG_ARC_TIMER)        += arc_timer.o
>  obj-$(CONFIG_AG101P_TIMER) += ag101p_timer.o
>  obj-$(CONFIG_AE3XX_TIMER) += ae3xx_timer.o
> +obj-$(CONFIG_ROCKCHIP_TIMER) += rockchip_timer.o
> diff --git a/drivers/timer/rockchip_timer.c b/drivers/timer/rockchip_timer.c
> new file mode 100644
> index 0000000..bffb630
> --- /dev/null
> +++ b/drivers/timer/rockchip_timer.c
> @@ -0,0 +1,105 @@
> +/*
> + * Copyright (C) 2017 Theobroma Systems Design und Consulting GmbH
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#define DEBUG

Can we drop this?

> +
> +#include <common.h>
> +#include <dm.h>
> +#include <mapmem.h>
> +#include <asm/arch/timer.h>
> +#include <dt-structs.h>
> +#include <timer.h>
> +#include <asm/io.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#if CONFIG_IS_ENABLED(OF_PLATDATA)
> +struct rockchip_timer_plat {
> +       struct dtd_rockchip_rk3368_timer dtd;
> +};
> +#endif
> +
> +/* Driver private data. Contains timer id. Could be either 0 or 1. */
> +struct rockchip_timer_priv {
> +       struct rk_timer *timer;
> +};
> +
> +static int rockchip_timer_get_count(struct udevice *dev, u64 *count)
> +{
> +       struct rockchip_timer_priv *priv = dev_get_priv(dev);
> +       uint64_t timebase_h, timebase_l;
> +       uint64_t cntr;
> +
> +       timebase_l = readl(&priv->timer->timer_curr_value0);
> +       timebase_h = readl(&priv->timer->timer_curr_value1);
> +
> +       /* timers are down-counting */
> +       cntr = timebase_h << 32 | timebase_l;
> +       *count = 0xffffffffull - cntr;
> +       return 0;
> +}
> +
> +static int rockchip_clk_ofdata_to_platdata(struct udevice *dev)
> +{
> +#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> +       struct rockchip_timer_priv *priv = dev_get_priv(dev);
> +
> +       priv->timer = (struct rk_timer *)devfdt_get_addr(dev);
> +#endif
> +
> +       return 0;
> +}
> +
> +static int rockchip_timer_start(struct udevice *dev)
> +{
> +       struct rockchip_timer_priv *priv = dev_get_priv(dev);
> +
> +       writel(0, &priv->timer->timer_ctrl_reg);
> +       writel(0xffffffff, &priv->timer->timer_load_count0);
> +       writel(0xffffffff, &priv->timer->timer_load_count1);
> +       writel(1, &priv->timer->timer_ctrl_reg);
> +
> +       return 0;
> +}
> +
> +static int rockchip_timer_probe(struct udevice *dev)
> +{
> +       struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> +       const u32 MHz = 1000000;
> +
> +#if CONFIG_IS_ENABLED(OF_PLATDATA)
> +       struct rockchip_timer_priv *priv = dev_get_priv(dev);
> +       struct rockchip_timer_plat *plat = dev_get_platdata(dev);
> +
> +       priv->timer = map_sysmem(plat->dtd.reg[1], plat->dtd.reg[3]);
> +#endif
> +       uc_priv->clock_rate = 24 * MHz;

If it is not enabled, doesn't this come from the DT?

> +
> +       return rockchip_timer_start(dev);
> +}
> +
> +static const struct timer_ops rockchip_timer_ops = {
> +       .get_count = rockchip_timer_get_count,
> +};
> +
> +static const struct udevice_id rockchip_timer_ids[] = {
> +       { .compatible = "rockchip,rk3368-timer" },
> +       {}
> +};
> +
> +U_BOOT_DRIVER(arc_timer) = {
> +       .name   = "rockchip_rk3368_timer",
> +       .id     = UCLASS_TIMER,
> +       .of_match = rockchip_timer_ids,
> +       .probe = rockchip_timer_probe,
> +       .ops    = &rockchip_timer_ops,
> +       .flags = DM_FLAG_PRE_RELOC,
> +       .priv_auto_alloc_size = sizeof(struct rockchip_timer_priv),
> +#if CONFIG_IS_ENABLED(OF_PLATDATA)
> +       .platdata_auto_alloc_size = sizeof(struct rockchip_timer_plat),
> +#endif
> +       .ofdata_to_platdata = rockchip_clk_ofdata_to_platdata,
> +};
> --
> 2.1.4
>

Regards,
Simon