[U-Boot] [RFC PATCH 0/6] rockchip: rk3368: remove secure timer usage and use DM timer
Trying to answer Simon's question whether the address of the secure timer (for initialising stimer1 and starting up the ARMv8 generic timer) can be obtained from the DTS, here's a series that tries to give an answer.
To summarise this answer in plain English: - The answer to the original question is: "no, but..." - The "but" is what's implemented here: we don't need the ARMv8 generic timer ticking in U-Boot, so we won't have to initialise it at all (this removing the need to obtain the address for stimer1). - We also have a "however": the size of the TPL binary increases by approx. 800 bytes, as we need the DM timer support.
This series is based on-top-of my RK3368 enablement series.
If we go ahead with merging this, then I'll have to add support for the RK3399 as well...
Philipp Tomsich (6): timer: add OF_PLATDATA support for timer-uclass dm: timer: normalise SPL and TPL support rockchip: timer: add device-model timer driver for RK3368 (and similar) dts: rk3368: make timer0 accessible for SPL and TPL rockchip: lion-rk3368: defconfig: enable DM timer for all stages rockchip: rk3368: remove setup of secure timer from TPL/SPL
arch/arm/cpu/armv8/Makefile | 2 + arch/arm/dts/rk3368-lion-u-boot.dtsi | 2 +- arch/arm/dts/rk3368.dtsi | 2 +- arch/arm/mach-rockchip/rk3368-board-spl.c | 20 ------ arch/arm/mach-rockchip/rk3368-board-tpl.c | 19 ------ common/spl/Kconfig | 8 --- configs/chromebook_link64_defconfig | 2 +- configs/lion-rk3368_defconfig | 4 ++ configs/qemu-x86_64_defconfig | 2 +- drivers/Makefile | 3 +- drivers/timer/Kconfig | 25 +++++++ drivers/timer/Makefile | 3 +- drivers/timer/rockchip_timer.c | 105 ++++++++++++++++++++++++++++++ drivers/timer/timer-uclass.c | 6 +- 14 files changed, 148 insertions(+), 55 deletions(-) create mode 100644 drivers/timer/rockchip_timer.c
The timer-uclass depends on full OF_CONTROL through its interrogation of /chosen and the code to determine the clock-frequency.
For the OF_PLATDATA case, these code-paths are disabled and it becomes the timer driver's responsibility to correctly set the clock-frequency in the uclass priv-data.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
drivers/timer/timer-uclass.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c index ec10b28..f5697d2 100644 --- a/drivers/timer/timer-uclass.c +++ b/drivers/timer/timer-uclass.c @@ -42,6 +42,7 @@ unsigned long notrace timer_get_rate(struct udevice *dev)
static int timer_pre_probe(struct udevice *dev) { +#if !CONFIG_IS_ENABLED(OF_PLATDATA) struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); struct clk timer_clk; int err; @@ -56,6 +57,7 @@ static int timer_pre_probe(struct udevice *dev) } else uc_priv->clock_rate = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), "clock-frequency", 0); +#endif
return 0; } @@ -83,14 +85,16 @@ int notrace dm_timer_init(void) { const void *blob = gd->fdt_blob; struct udevice *dev = NULL; - int node; + int node = -ENOENT; int ret;
if (gd->timer) return 0;
+#if !CONFIG_IS_ENABLED(OF_PLATDATA) /* Check for a chosen timer to be used for tick */ node = fdtdec_get_chosen_node(blob, "tick-timer"); +#endif if (node < 0) { /* No chosen timer, trying first available timer */ ret = uclass_first_device_err(UCLASS_TIMER, &dev);
On 28 July 2017 at 10:31, Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
The timer-uclass depends on full OF_CONTROL through its interrogation of /chosen and the code to determine the clock-frequency.
For the OF_PLATDATA case, these code-paths are disabled and it becomes the timer driver's responsibility to correctly set the clock-frequency in the uclass priv-data.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
drivers/timer/timer-uclass.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
To fully support DM timer in SPL and TPL, we need a few things cleaned up and normalised: - inclusion of the uclass and drivers should be an all-or-nothing decision for each stage and under control of $(SPL_TPL_)TIMER instead of having the two-level configuration with TIMER and $(SPL_TPL_)TIMER_SUPPORT - when $(SPL_TPL_)TIMER is enabled, the ARMv8 generic timer code can not be compiled in
This normalises configuration to $(SPL_TPL_)TIMER and moves the config options to drivers/timer/Kconfig (and cleans up the collateral damage to some defconfigs that had SPL_TIMER_SUPPORT enabled).
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
arch/arm/cpu/armv8/Makefile | 2 ++ common/spl/Kconfig | 8 -------- configs/chromebook_link64_defconfig | 2 +- configs/qemu-x86_64_defconfig | 2 +- drivers/Makefile | 3 +-- drivers/timer/Kconfig | 18 ++++++++++++++++++ drivers/timer/Makefile | 2 +- 7 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile index 64f35f1..be3b39e 100644 --- a/arch/arm/cpu/armv8/Makefile +++ b/arch/arm/cpu/armv8/Makefile @@ -8,7 +8,9 @@ extra-y := start.o
obj-y += cpu.o +ifndef CONFIG_$(SPL_TPL_)TIMER obj-y += generic_timer.o +endif obj-y += cache_v8.o obj-y += cache.o obj-y += tlb.o diff --git a/common/spl/Kconfig b/common/spl/Kconfig index 64f9e1f..a3c0cc4 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -593,14 +593,6 @@ config SPL_SPI_SUPPORT enable SPI drivers that are needed for other purposes also, such as a SPI PMIC.
-config SPL_TIMER_SUPPORT - bool "Support timer drivers" - help - Enable support for timer drivers in SPL. These can be used to get - a timer value when in SPL, or perhaps for implementing a delay - function. This enables the drivers in drivers/timer as part of an - SPL build. - config SPL_USB_HOST_SUPPORT bool "Support USB host drivers" help diff --git a/configs/chromebook_link64_defconfig b/configs/chromebook_link64_defconfig index 11ceb76..5cf409c 100644 --- a/configs/chromebook_link64_defconfig +++ b/configs/chromebook_link64_defconfig @@ -28,7 +28,7 @@ CONFIG_SPL_NET_SUPPORT=y CONFIG_SPL_PCI_SUPPORT=y CONFIG_SPL_PCH_SUPPORT=y CONFIG_SPL_RTC_SUPPORT=y -CONFIG_SPL_TIMER_SUPPORT=y +CONFIG_SPL_TIMER=y CONFIG_HUSH_PARSER=y CONFIG_CMD_CPU=y # CONFIG_CMD_BOOTEFI_HELLO_COMPILE is not set diff --git a/configs/qemu-x86_64_defconfig b/configs/qemu-x86_64_defconfig index fc1c70d..9517001 100644 --- a/configs/qemu-x86_64_defconfig +++ b/configs/qemu-x86_64_defconfig @@ -29,7 +29,7 @@ CONFIG_SPL_NET_SUPPORT=y CONFIG_SPL_PCI_SUPPORT=y CONFIG_SPL_PCH_SUPPORT=y CONFIG_SPL_RTC_SUPPORT=y -CONFIG_SPL_TIMER_SUPPORT=y +CONFIG_SPL_TIMER=y CONFIG_HUSH_PARSER=y CONFIG_CMD_CPU=y # CONFIG_CMD_BOOTEFI_HELLO_COMPILE is not set diff --git a/drivers/Makefile b/drivers/Makefile index b98550e..9d4680a 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_$(SPL_TPL_)RAM) += ram/ obj-$(CONFIG_$(SPL_TPL_)SERIAL_SUPPORT) += serial/ obj-$(CONFIG_$(SPL_TPL_)SPI_FLASH_SUPPORT) += mtd/spi/ obj-$(CONFIG_$(SPL_TPL_)SPI_SUPPORT) += spi/ +obj-$(CONFIG_$(SPL_TPL_)TIMER) += timer/
ifndef CONFIG_TPL_BUILD ifdef CONFIG_SPL_BUILD @@ -38,7 +39,6 @@ obj-$(CONFIG_SPL_USBETH_SUPPORT) += net/phy/ obj-$(CONFIG_SPL_PCI_SUPPORT) += pci/ obj-$(CONFIG_SPL_PCH_SUPPORT) += pch/ obj-$(CONFIG_SPL_RTC_SUPPORT) += rtc/ -obj-$(CONFIG_SPL_TIMER_SUPPORT) += timer/ obj-$(CONFIG_SPL_MUSB_NEW_SUPPORT) += usb/musb-new/ obj-$(CONFIG_SPL_USB_GADGET_SUPPORT) += usb/gadget/ obj-$(CONFIG_SPL_USB_GADGET_SUPPORT) += usb/gadget/udc/ @@ -82,7 +82,6 @@ obj-y += scsi/ obj-y += sound/ obj-y += spmi/ obj-y += sysreset/ -obj-y += timer/ obj-y += tpm/ obj-y += video/ obj-y += watchdog/ diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index 17e7dfe..fb5af4d 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -9,6 +9,24 @@ config 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.
+config SPL_TIMER + bool "Enable driver model for timer drivers in SPL" + depends on TIMER && SPL + help + Enable support for timer drivers in SPL. These can be used to get + a timer value when in SPL, or perhaps for implementing a delay + function. This enables the drivers in drivers/timer as part of an + SPL build. + +config TPL_TIMER + bool "Enable driver model for timer drivers in TPL" + depends on TIMER && TPL + help + Enable support for timer drivers in TPL. These can be used to get + a timer value when in TPL, or perhaps for implementing a delay + function. This enables the drivers in drivers/timer as part of an + TPL build. + config TIMER_EARLY bool "Allow timer to be used early in U-Boot" depends on TIMER diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile index ced7bd6..d16ea53 100644 --- a/drivers/timer/Makefile +++ b/drivers/timer/Makefile @@ -4,7 +4,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-obj-$(CONFIG_TIMER) += timer-uclass.o +obj-y += timer-uclass.o obj-$(CONFIG_ALTERA_TIMER) += altera_timer.o obj-$(CONFIG_SANDBOX_TIMER) += sandbox_timer.o obj-$(CONFIG_X86_TSC_TIMER) += tsc_timer.o
On 28 July 2017 at 10:31, Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
To fully support DM timer in SPL and TPL, we need a few things cleaned up and normalised:
- inclusion of the uclass and drivers should be an all-or-nothing decision for each stage and under control of $(SPL_TPL_)TIMER instead of having the two-level configuration with TIMER and $(SPL_TPL_)TIMER_SUPPORT
- when $(SPL_TPL_)TIMER is enabled, the ARMv8 generic timer code can not be compiled in
This normalises configuration to $(SPL_TPL_)TIMER and moves the config options to drivers/timer/Kconfig (and cleans up the collateral damage to some defconfigs that had SPL_TIMER_SUPPORT enabled).
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
arch/arm/cpu/armv8/Makefile | 2 ++ common/spl/Kconfig | 8 -------- configs/chromebook_link64_defconfig | 2 +- configs/qemu-x86_64_defconfig | 2 +- drivers/Makefile | 3 +-- drivers/timer/Kconfig | 18 ++++++++++++++++++ drivers/timer/Makefile | 2 +- 7 files changed, 24 insertions(+), 13 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
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@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
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 + 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 + +#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; + + 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, +};
On 28 July 2017 at 10:31, Philipp Tomsich philipp.tomsich@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@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@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 TIMERhelpSelect 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
On 01 Aug 2017, at 11:48, Simon Glass sjg@chromium.org wrote:
On 28 July 2017 at 10:31, Philipp Tomsich philipp.tomsich@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@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@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 TIMERhelpSelect this to enable support for the timer block found onI'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?
Unfortunately not, as the DTS neither defines a parent-clk nor a clock-frequency. This should indeed not be here — I’ll add a change that fixes the DTS.
Thanks for highlighting this!
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
To use it with the DM timer driver in SPL and TPL, timer0 needs to be marked as pre-reloc.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
arch/arm/dts/rk3368-lion-u-boot.dtsi | 2 +- arch/arm/dts/rk3368.dtsi | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/dts/rk3368-lion-u-boot.dtsi b/arch/arm/dts/rk3368-lion-u-boot.dtsi index 024eb2e..74c6e55 100644 --- a/arch/arm/dts/rk3368-lion-u-boot.dtsi +++ b/arch/arm/dts/rk3368-lion-u-boot.dtsi @@ -85,7 +85,7 @@ }; };
-&stimer1 { +&timer0 { u-boot,dm-pre-reloc; };
diff --git a/arch/arm/dts/rk3368.dtsi b/arch/arm/dts/rk3368.dtsi index 22fb7e7..b4f4f61 100644 --- a/arch/arm/dts/rk3368.dtsi +++ b/arch/arm/dts/rk3368.dtsi @@ -687,7 +687,7 @@ status = "disabled"; };
- timer@ff810000 { + timer0: timer@ff810000 { compatible = "rockchip,rk3368-timer", "rockchip,rk3288-timer"; reg = <0x0 0xff810000 0x0 0x20>; interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>;
On 28 July 2017 at 10:31, Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
To use it with the DM timer driver in SPL and TPL, timer0 needs to be marked as pre-reloc.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
arch/arm/dts/rk3368-lion-u-boot.dtsi | 2 +- arch/arm/dts/rk3368.dtsi | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
There is no reasonably robust way (this will be needed so early that diagnostics will be limited) to specify the base-address of the secure timer through the DTS for TPL and SPL. In order to allow us a cleaner way to structure our SPL and TPL stage, we now move to a DM timer driver.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
configs/lion-rk3368_defconfig | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/configs/lion-rk3368_defconfig b/configs/lion-rk3368_defconfig index e3b8df9..1786acc 100644 --- a/configs/lion-rk3368_defconfig +++ b/configs/lion-rk3368_defconfig @@ -85,6 +85,10 @@ CONFIG_DEBUG_UART_ANNOUNCE=y CONFIG_DEBUG_UART_SKIP_INIT=y CONFIG_ROCKCHIP_SPI=y CONFIG_SYSRESET=y +CONFIG_TIMER=y +CONFIG_SPL_TIMER=y +CONFIG_TPL_TIMER=y +CONFIG_ROCKCHIP_TIMER=y CONFIG_USE_TINY_PRINTF=y CONFIG_SPL_TINY_MEMSET=y CONFIG_LZO=y
On 28 July 2017 at 10:31, Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
There is no reasonably robust way (this will be needed so early that diagnostics will be limited) to specify the base-address of the secure timer through the DTS for TPL and SPL. In order to allow us a cleaner way to structure our SPL and TPL stage, we now move to a DM timer driver.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
configs/lion-rk3368_defconfig | 4 ++++ 1 file changed, 4 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
When using DM timers w/ the timer0 block within the RK3368, we no longer depend on the ARMv8 generic timer counting. This allows us to drop the secure timer initialisation from the TPL and SPL stages.
The secure timer will later be set up by ATF, which starts the ARMv8 generic timer. Thus, there will be a dependency from Linux to the ATF through the ARMv8 generic timer... this seems reasonable, as Linux will require the ATF (and PSCI) to start up the secondary cores anyway (in other words: we don't add any new dependencies).
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
---
arch/arm/mach-rockchip/rk3368-board-spl.c | 20 -------------------- arch/arm/mach-rockchip/rk3368-board-tpl.c | 19 ------------------- 2 files changed, 39 deletions(-)
diff --git a/arch/arm/mach-rockchip/rk3368-board-spl.c b/arch/arm/mach-rockchip/rk3368-board-spl.c index 691db41..cabf344 100644 --- a/arch/arm/mach-rockchip/rk3368-board-spl.c +++ b/arch/arm/mach-rockchip/rk3368-board-spl.c @@ -19,23 +19,6 @@
DECLARE_GLOBAL_DATA_PTR;
-/* - * The ARMv8 generic timer uses the STIMER1 as its clock-source. - * Set up the STIMER1 to free-running (i.e. auto-reload) to start - * the generic timer counting (if we don't do this, udelay will not - * work and block indefinitively). - */ -static void secure_timer_init(void) -{ - struct rk_timer * const stimer1 = - (struct rk_timer * const)0xff830020; - const u32 TIMER_EN = BIT(0); - - writel(~0u, &stimer1->timer_load_count0); - writel(~0u, &stimer1->timer_load_count1); - writel(TIMER_EN, &stimer1->timer_ctrl_reg); -} - void board_debug_uart_init(void) { } @@ -52,9 +35,6 @@ void board_init_f(ulong dummy) hang(); }
- /* Make sure the ARMv8 generic timer counts */ - secure_timer_init(); - /* Set up our preloader console */ ret = uclass_get_device(UCLASS_PINCTRL, 0, &pinctrl); if (ret) { diff --git a/arch/arm/mach-rockchip/rk3368-board-tpl.c b/arch/arm/mach-rockchip/rk3368-board-tpl.c index a544a0f..e2e2108 100644 --- a/arch/arm/mach-rockchip/rk3368-board-tpl.c +++ b/arch/arm/mach-rockchip/rk3368-board-tpl.c @@ -21,23 +21,6 @@ DECLARE_GLOBAL_DATA_PTR;
/* - * The ARMv8 generic timer uses the STIMER1 as its clock-source. - * Set up the STIMER1 to free-running (i.e. auto-reload) to start - * the generic timer counting (if we don't do this, udelay will not - * work and block indefinitively). - */ -static void secure_timer_init(void) -{ - struct rk_timer * const stimer1 = - (struct rk_timer * const)0xff830020; - const u32 TIMER_EN = BIT(0); - - writel(~0u, &stimer1->timer_load_count0); - writel(~0u, &stimer1->timer_load_count1); - writel(TIMER_EN, &stimer1->timer_ctrl_reg); -} - -/* * The SPL (and also the full U-Boot stage on the RK3368) will run in * secure mode (i.e. EL3) and an ATF will eventually be booted before * starting up the operating system... so we can initialize the SGRF @@ -148,8 +131,6 @@ void board_init_f(ulong dummy) hang(); }
- /* Make sure the ARMv8 generic timer counts */ - secure_timer_init(); /* Reset security, so we can use DMA in the MMC drivers */ sgrf_init();
On 28 July 2017 at 10:31, Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
When using DM timers w/ the timer0 block within the RK3368, we no longer depend on the ARMv8 generic timer counting. This allows us to drop the secure timer initialisation from the TPL and SPL stages.
The secure timer will later be set up by ATF, which starts the ARMv8 generic timer. Thus, there will be a dependency from Linux to the ATF through the ARMv8 generic timer... this seems reasonable, as Linux will require the ATF (and PSCI) to start up the secondary cores anyway (in other words: we don't add any new dependencies).
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
arch/arm/mach-rockchip/rk3368-board-spl.c | 20 -------------------- arch/arm/mach-rockchip/rk3368-board-tpl.c | 19 ------------------- 2 files changed, 39 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Hi Philipp, Simon,
I still think we should have a option to no involve so much framework thing in
very early boot stage like TPL/SPL, for those chipset with very little sram onchip.
Andy send some patches for rk3036 recently, because it get out of memory in SPL
and not able to boot. Most of people do not notice this but please leave a option for
those devices. The timer init should always happen very early like SPL/TPL, and I don't
think the DM is a must for it.
Thanks, - Kever On 07/29/2017 12:31 AM, Philipp Tomsich wrote:
Trying to answer Simon's question whether the address of the secure timer (for initialising stimer1 and starting up the ARMv8 generic timer) can be obtained from the DTS, here's a series that tries to give an answer.
To summarise this answer in plain English:
- The answer to the original question is: "no, but..."
- The "but" is what's implemented here: we don't need the ARMv8 generic timer ticking in U-Boot, so we won't have to initialise it at all (this removing the need to obtain the address for stimer1).
- We also have a "however": the size of the TPL binary increases by approx. 800 bytes, as we need the DM timer support.
This series is based on-top-of my RK3368 enablement series.
If we go ahead with merging this, then I'll have to add support for the RK3399 as well...
Philipp Tomsich (6): timer: add OF_PLATDATA support for timer-uclass dm: timer: normalise SPL and TPL support rockchip: timer: add device-model timer driver for RK3368 (and similar) dts: rk3368: make timer0 accessible for SPL and TPL rockchip: lion-rk3368: defconfig: enable DM timer for all stages rockchip: rk3368: remove setup of secure timer from TPL/SPL
arch/arm/cpu/armv8/Makefile | 2 + arch/arm/dts/rk3368-lion-u-boot.dtsi | 2 +- arch/arm/dts/rk3368.dtsi | 2 +- arch/arm/mach-rockchip/rk3368-board-spl.c | 20 ------ arch/arm/mach-rockchip/rk3368-board-tpl.c | 19 ------ common/spl/Kconfig | 8 --- configs/chromebook_link64_defconfig | 2 +- configs/lion-rk3368_defconfig | 4 ++ configs/qemu-x86_64_defconfig | 2 +- drivers/Makefile | 3 +- drivers/timer/Kconfig | 25 +++++++ drivers/timer/Makefile | 3 +- drivers/timer/rockchip_timer.c | 105 ++++++++++++++++++++++++++++++ drivers/timer/timer-uclass.c | 6 +- 14 files changed, 148 insertions(+), 55 deletions(-) create mode 100644 drivers/timer/rockchip_timer.c
Kever,
This patchset does not force the use of the DM timer for any chipsets/boards. It is essentially opt-in and (for now—i.e. until we enable it for the RK3399) it is enabled for the RK3368 only.
Note that this does not currently target any ARMv7 devices, as it is meant to decouple the ARMv8 generic timer startup (i.e. starting the stimer) from U-Boot.
Regards, Philipp.
On 02 Aug 2017, at 07:05, Kever Yang kever.yang@rock-chips.com wrote:
Hi Philipp, Simon,
I still think we should have a option to no involve so much framework thing in
very early boot stage like TPL/SPL, for those chipset with very little sram onchip.
Andy send some patches for rk3036 recently, because it get out of memory in SPL
and not able to boot. Most of people do not notice this but please leave a option for
those devices. The timer init should always happen very early like SPL/TPL, and I don't
think the DM is a must for it.
Thanks,
- Kever
On 07/29/2017 12:31 AM, Philipp Tomsich wrote:
Trying to answer Simon's question whether the address of the secure timer (for initialising stimer1 and starting up the ARMv8 generic timer) can be obtained from the DTS, here's a series that tries to give an answer.
To summarise this answer in plain English:
- The answer to the original question is: "no, but..."
- The "but" is what's implemented here: we don't need the ARMv8 generic timer ticking in U-Boot, so we won't have to initialise it at all (this removing the need to obtain the address for stimer1).
- We also have a "however": the size of the TPL binary increases by approx. 800 bytes, as we need the DM timer support.
This series is based on-top-of my RK3368 enablement series.
If we go ahead with merging this, then I'll have to add support for the RK3399 as well...
Philipp Tomsich (6): timer: add OF_PLATDATA support for timer-uclass dm: timer: normalise SPL and TPL support rockchip: timer: add device-model timer driver for RK3368 (and similar) dts: rk3368: make timer0 accessible for SPL and TPL rockchip: lion-rk3368: defconfig: enable DM timer for all stages rockchip: rk3368: remove setup of secure timer from TPL/SPL
arch/arm/cpu/armv8/Makefile | 2 + arch/arm/dts/rk3368-lion-u-boot.dtsi | 2 +- arch/arm/dts/rk3368.dtsi | 2 +- arch/arm/mach-rockchip/rk3368-board-spl.c | 20 ------ arch/arm/mach-rockchip/rk3368-board-tpl.c | 19 ------ common/spl/Kconfig | 8 --- configs/chromebook_link64_defconfig | 2 +- configs/lion-rk3368_defconfig | 4 ++ configs/qemu-x86_64_defconfig | 2 +- drivers/Makefile | 3 +- drivers/timer/Kconfig | 25 +++++++ drivers/timer/Makefile | 3 +- drivers/timer/rockchip_timer.c | 105 ++++++++++++++++++++++++++++++ drivers/timer/timer-uclass.c | 6 +- 14 files changed, 148 insertions(+), 55 deletions(-) create mode 100644 drivers/timer/rockchip_timer.c
Hi,
On 2 August 2017 at 05:09, Dr. Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
Kever,
This patchset does not force the use of the DM timer for any chipsets/boards. It is essentially opt-in and (for now—i.e. until we enable it for the RK3399) it is enabled for the RK3368 only.
Note that this does not currently target any ARMv7 devices, as it is meant to decouple the ARMv8 generic timer startup (i.e. starting the stimer) from U-Boot.
Regards, Philipp.
On 02 Aug 2017, at 07:05, Kever Yang kever.yang@rock-chips.com wrote:
Hi Philipp, Simon,
I still think we should have a option to no involve so much framework thing in
very early boot stage like TPL/SPL, for those chipset with very little sram onchip.
Andy send some patches for rk3036 recently, because it get out of memory in SPL
and not able to boot. Most of people do not notice this but please leave a option for
those devices. The timer init should always happen very early like SPL/TPL, and I don't
think the DM is a must for it.
Just a note on this. There is a CONFIG_TIMER_EARLY option available which is specifically designed to provide a timer before DM is ready.
Thanks,
- Kever
On 07/29/2017 12:31 AM, Philipp Tomsich wrote:
Trying to answer Simon's question whether the address of the secure timer (for initialising stimer1 and starting up the ARMv8 generic timer) can be obtained from the DTS, here's a series that tries to give an answer.
To summarise this answer in plain English:
- The answer to the original question is: "no, but..."
- The "but" is what's implemented here: we don't need the ARMv8 generic timer ticking in U-Boot, so we won't have to initialise it at all (this removing the need to obtain the address for stimer1).
- We also have a "however": the size of the TPL binary increases by approx. 800 bytes, as we need the DM timer support.
This series is based on-top-of my RK3368 enablement series.
If we go ahead with merging this, then I'll have to add support for the RK3399 as well...
Philipp Tomsich (6): timer: add OF_PLATDATA support for timer-uclass dm: timer: normalise SPL and TPL support rockchip: timer: add device-model timer driver for RK3368 (and similar) dts: rk3368: make timer0 accessible for SPL and TPL rockchip: lion-rk3368: defconfig: enable DM timer for all stages rockchip: rk3368: remove setup of secure timer from TPL/SPL
arch/arm/cpu/armv8/Makefile | 2 + arch/arm/dts/rk3368-lion-u-boot.dtsi | 2 +- arch/arm/dts/rk3368.dtsi | 2 +- arch/arm/mach-rockchip/rk3368-board-spl.c | 20 ------ arch/arm/mach-rockchip/rk3368-board-tpl.c | 19 ------ common/spl/Kconfig | 8 --- configs/chromebook_link64_defconfig | 2 +- configs/lion-rk3368_defconfig | 4 ++ configs/qemu-x86_64_defconfig | 2 +- drivers/Makefile | 3 +- drivers/timer/Kconfig | 25 +++++++ drivers/timer/Makefile | 3 +- drivers/timer/rockchip_timer.c | 105 ++++++++++++++++++++++++++++++ drivers/timer/timer-uclass.c | 6 +- 14 files changed, 148 insertions(+), 55 deletions(-) create mode 100644 drivers/timer/rockchip_timer.c
Regards, Simon
participants (4)
-
Dr. Philipp Tomsich -
Kever Yang -
Philipp Tomsich
-
Simon Glass