ARM: davinci: modernize the timer support
Subsystems (6)
v2 cover ARM: davinci: modernize the timer support
From: Bartosz Golaszewski <[email protected]>
This series removes the legacy timer code from mach-davinci in favor of a new clocksource driver it introduces.
The first patch fixes a device tree bug that's been around for a while. Unfortunately any systems shipped with the buggy device will stop booting once they switch to the new clocksource driver.
Patch 2 adds a new clocksource driver for davinci.
Patch 3 enables the new driver for device-tree based systems.
Patch 4 adds a WARN_ON() to the machine code of all davinci boards which is triggered if clk_get() for the timer clock fails. This is needed as the new driver expects the clock to be functional and doesn't check it.
Patches 5-6 and 8-11 switch the board files to using the new clocksource driver while patch 7 moves some necessary defines to a different place since we'll be removing the file that contains them.
Patch 12 removes legacy timer code.
v1 -> v2:
- changed the license to GPL-only
- dropped regmap usage due to the additional code paths making the code too slow
- minor code rearrangement
- fixed the da830 timer configuration
- correctly assign the timer halfs to clocksource and clockevent
- sqashed the commits adding WARN() when clk_get() fails
- used WARN_ON() and added a return when clk_get() fails to avoid entering the timer driver without a valid clock
- request the register range before iomapping it
- simplified the configuration structure (dropped the additional cmp struct)
Bartosz Golaszewski (12):
ARM: dts: da850: fix interrupt numbers for clocksource
clocksource: davinci-timer: new driver
ARM: davinci: enable the clocksource driver for DT mode
ARM: davinci: WARN_ON() if clk_get() fails
ARM: davinci: da850: switch to using the clocksource driver
ARM: davinci: da830: switch to using the clocksource driver
ARM: davinci: move timer definitions to davinci.h
ARM: davinci: dm355: switch to using the clocksource driver
ARM: davinci: dm365: switch to using the clocksource driver
ARM: davinci: dm644x: switch to using the clocksource driver
ARM: davinci: dm646x: switch to using the clocksource driver
ARM: davinci: remove legacy timer support
arch/arm/Kconfig | 1 + arch/arm/boot/dts/da850.dtsi | 2 +- arch/arm/mach-davinci/Makefile | 3 +- arch/arm/mach-davinci/da830.c | 53 ++- arch/arm/mach-davinci/da850.c | 60 ++- arch/arm/mach-davinci/davinci.h | 3 + arch/arm/mach-davinci/devices-da8xx.c | 1 - arch/arm/mach-davinci/devices.c | 19 - arch/arm/mach-davinci/dm355.c | 40 +- arch/arm/mach-davinci/dm365.c | 34 +- arch/arm/mach-davinci/dm644x.c | 40 +- arch/arm/mach-davinci/dm646x.c | 40 +- arch/arm/mach-davinci/include/mach/common.h | 17 - arch/arm/mach-davinci/include/mach/time.h | 35 -- arch/arm/mach-davinci/time.c | 414 ------------------- drivers/clocksource/Kconfig | 5 + drivers/clocksource/Makefile | 1 + drivers/clocksource/timer-davinci.c | 431 ++++++++++++++++++++ include/clocksource/timer-davinci.h | 44 ++ 19 files changed, 647 insertions(+), 596 deletions(-) delete mode 100644 arch/arm/mach-davinci/include/mach/time.h delete mode 100644 arch/arm/mach-davinci/time.c create mode 100644 drivers/clocksource/timer-davinci.c create mode 100644 include/clocksource/timer-davinci.h
Signature · 1 line
2.20.1
PATCH v2 1/12 ARM: dts: da850: fix interrupt numbers for clocksource
From: Bartosz Golaszewski <[email protected]>
The timer interrupts specified in commit 3652e2741f42 ("ARM: dts:
da850: Add clocks") are wrong but since the current timer code hard-codes them, the bug was never spotted.
This patch must go into stable since, once we introduce a proper clocksource driver, devices with buggy device tree will stop booting.
--- arch/arm/boot/dts/da850.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/da850.dtsi +1 -1
@@ -476,7 +476,7 @@ clocksource: timer@20000 { compatible = "ti,da830-timer"; reg = <0x20000 0x1000>; - interrupts = <12>, <13>; + interrupts = <21>, <22>; interrupt-names = "tint12", "tint34"; clocks = <&pll0_auxclk>; };
Signature · 1 line
2.20.1
Re: [PATCH v2 01/12] ARM: dts: da850: fix interrupt numbers for clocksource
Quoted 6 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
The timer interrupts specified in commit
3652e2741f42("ARM: dts:
da850: Add clocks") are wrong but since the current timer code hard-codes them, the bug was never spotted.
Quoted 7 lines
This patch must go into stable since, once we introduce a proper clocksource driver, devices with buggy device tree will stop booting.
Fixes 3652e2741f42 ("ARM: dts: da850: Add clocks")Signed-off-by Bartosz Golaszewski <[email protected]>---
Re: [PATCH v2 01/12] ARM: dts: da850: fix interrupt numbers for clocksource
Quoted 10 lines · David Lechner
Quoted 9 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
The timer interrupts specified in commit
3652e2741f42("ARM: dts:
da850: Add clocks") are wrong but since the current timer code hard-codes them, the bug was never spotted.This patch must go into stable since, once we introduce a proper clocksource driver, devices with buggy device tree will stop booting.
Quoted 6 lines
Quoted 4 lines
Fixes 3652e2741f42 ("ARM: dts: da850: Add clocks")Signed-off-by Bartosz Golaszewski <[email protected]>---
Reviewed-by David Lechner <[email protected]>
This patch is already in ARM SoC. Should be soon in Linus's tree too.
Thanks,
Sekhar
PATCH v2 2/12 clocksource: davinci-timer: new driver
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Currently the clocksource and clockevent support for davinci platforms lives in mach-davinci. It hard-codes many things, used global variables, implements functionalities unused by any platform and has code fragments scattered across many (often unrelated) files.
Implement a new, modern and simplified timer driver and put it into drivers/clocksource. We still need to support legacy board files so export a config structure and a function that allows machine code to register the timer.
We don't bother freeing resources on errors in davinci_timer_register() as the system won't boot without a timer anyway.
--- drivers/clocksource/Kconfig | 5 + drivers/clocksource/Makefile | 1 + drivers/clocksource/timer-davinci.c | 431 ++++++++++++++++++++++++++++ include/clocksource/timer-davinci.h | 44 +++ 4 files changed, 481 insertions(+) create mode 100644 drivers/clocksource/timer-davinci.c create mode 100644 include/clocksource/timer-davinci.h
drivers/clocksource/Kconfig +5 -0
@@ -42,6 +42,11 @@ config BCM_KONA_TIMER help Enables the support for the BCM Kona mobile timer driver. +config DAVINCI_TIMER + bool "Texas Instruments DaVinci timer driver" + help + Enables the support for the TI DaVinci timer driver. + config DIGICOLOR_TIMER bool "Digicolor timer driver" if COMPILE_TEST select CLKSRC_MMIO
drivers/clocksource/Makefile +1 -0
@@ -15,6 +15,7 @@ obj-$(CONFIG_SH_TIMER_TMU) += sh_tmu.o obj-$(CONFIG_EM_TIMER_STI) += em_sti.o obj-$(CONFIG_CLKBLD_I8253) += i8253.o obj-$(CONFIG_CLKSRC_MMIO) += mmio.o +obj-$(CONFIG_DAVINCI_TIMER) += timer-davinci.o obj-$(CONFIG_DIGICOLOR_TIMER) += timer-digicolor.o obj-$(CONFIG_OMAP_DM_TIMER) += timer-ti-dm.o obj-$(CONFIG_DW_APB_TIMER) += dw_apb_timer.o
drivers/clocksource/timer-davinci.c new file +431 -0
431 lines not on this page · raw message
include/clocksource/timer-davinci.h new file +44 -0
44 lines not on this page · raw message
Signature · 1 line
2.20.1
Re: [PATCH v2 02/12] clocksource: davinci-timer: new driver
Quoted 17 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Currently the clocksource and clockevent support for davinci platforms lives in mach-davinci. It hard-codes many things, used global variables, implements functionalities unused by any platform and has code fragments scattered across many (often unrelated) files.
Implement a new, modern and simplified timer driver and put it into
drivers/clocksource. We still need to support legacy board files so export a config structure and a function that allows machine code to register the timer.We don't bother freeing resources on errors in
davinci_timer_register()as the system won't boot without a timer anyway.Signed-off-by Bartosz Golaszewski <[email protected]>---
...
drivers/clocksource/timer-davinci.c line 1
Quoted 7 lines
drivers/clocksource/timer-davinci.c new file +1
-0@@ -0,0 +1,431 @@ +// SPDX-License-Identifier: GPL-2.0
GPL-2.0-only ?
Quoted 28 lines
drivers/clocksource/timer-davinci.c new file +28
-0+// +// TI DaVinci clocksource driver +// +// Copyright (C) 2019 Texas Instruments +// Author: Bartosz Golaszewski <[email protected]> +// (with some parts adopted from code by Kevin Hilman <[email protected]>) + +#include <linux/clk.h> +#include <linux/clockchips.h> +#include <linux/clocksource.h> +#include <linux/err.h> +#include <linux/interrupt.h> +#include <linux/kernel.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> +#include <linux/sched_clock.h> + +#include <clocksource/timer-davinci.h> + +#define DAVINCI_TIMER_REG_TIM12 0x10 +#define DAVINCI_TIMER_REG_TIM34 0x14 +#define DAVINCI_TIMER_REG_PRD12 0x18 +#define DAVINCI_TIMER_REG_PRD34 0x1c +#define DAVINCI_TIMER_REG_TCR 0x20 +#define DAVINCI_TIMER_REG_TGCR 0x24 + +#define DAVINCI_TIMER_TIMMODE_MASK GENMASK(3, 2) +#define DAVINCI_TIMER_RESET_MASK GENMASK(1, 0)
drivers/clocksource/timer-davinci.c line 37
Quoted 10 lines
drivers/clocksource/timer-davinci.c new file +8
-0+#define DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED BIT(2) +#define DAVINCI_TIMER_UNRESET GENMASK(1, 0) + +/* Shift depends on timer. */ +#define DAVINCI_TIMER_ENAMODE_MASK GENMASK(1, 0) +#define DAVINCI_TIMER_ENAMODE_DISABLED 0x00 +#define DAVINCI_TIMER_ENAMODE_ONESHOT BIT(0) +#define DAVINCI_TIMER_ENAMODE_PERIODIC BIT(1)
Are these 3 values essentially an enum of exclusive values? If so, the the BIT() macro doesn't seem right here. If they are flags, then BIT() is OK, but DAVINCI_TIMER_ENAMODE_DISABLED could be omitted.
Quoted 15 lines
drivers/clocksource/timer-davinci.c new file +15
-0+ +#define DAVINCI_TIMER_ENAMODE_SHIFT_TIM12 6 +#define DAVINCI_TIMER_ENAMODE_SHIFT_TIM34 22 + +#define DAVINCI_TIMER_MIN_DELTA 0x01 +#define DAVINCI_TIMER_MAX_DELTA 0xfffffffe + +#define DAVINCI_TIMER_CLKSRC_BITS 32 + +#define DAVINCI_TIMER_TGCR_DEFAULT \ + (DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET) + +enum { +DAVINCI_TIMER_MODE_DISABLED =0 , +DAVINCI_TIMER_MODE_ONESHOT ,
drivers/clocksource/timer-davinci.c
Quoted 10 lines
drivers/clocksource/timer-davinci.c new file +10
-0+ DAVINCI_TIMER_MODE_PERIODIC, +}; + +struct davinci_timer_data ; + +typedef void (*davinci_timer_set_period_func )(struct davinci_timer_data *, +unsigned int period); + +struct davinci_timer_regs { + unsigned int tim_off;> + unsigned int prd_off;
It is not obvious to me what the abbreviations tim and prd mean. Add comments or change the names?
Quoted 3 lines
drivers/clocksource/timer-davinci.c new file +3
-0+unsigned int enamode_shift; +}; +
...
drivers/clocksource/timer-davinci.c line 129
Quoted 5 lines
drivers/clocksource/timer-davinci.c new file +5
-0+static void davinci_timer_update (struct davinci_timer_data *timer, +unsigned int reg,unsigned int mask, +unsigned int val) +{ +unsigned int new, orig =davinci_timer_read (timer, reg);
Slightly more readable with function not in variable declaration?
Quoted 6 lines
drivers/clocksource/timer-davinci.c new file +6
-0+ + new = orig & ~mask; + new |= val & mask; + +davinci_timer_write (timer, reg, new); +}
...
drivers/clocksource/timer-davinci.c line 286
Quoted 11 lines
drivers/clocksource/timer-davinci.c new file +11
-0+int __init davinci_timer_register (struct clk *clk, +const struct davinci_timer_cfg *timer_cfg) +{ +struct davinci_timer_clocksource *clocksource; +struct davinci_timer_clockevent *clockevent; +void __iomem *base; +int rv; + + rv =clk_prepare_enable (clk); +if (rv) { + pr_err("%s: Unable to prepare and enable the timer clock\n",
use pr_fmt marco to simplify? e.g. at the top of the file...
#define pr_fmt(fmt) "%s: " fmt "\n", __func__
Then just
pr_err("Unable to prepare and enable the timer clock");Quoted 3 lines
drivers/clocksource/timer-davinci.c new file +3
-0+ __func__); +return rv; + }
...
include/clocksource/timer-davinci.h line 1
Quoted 7 lines
include/clocksource/timer-davinci.h new file +1
-0@@ -0,0 +1,44 @@ +/* SPDX-License-Identifier: GPL-2.0+ */
Seems odd to have the header GPL-2.0+ when the c file GPL-2.0-only
Quoted 5 lines
include/clocksource/timer-davinci.h new file +5
-0+/* +* TI DaVinci clocksource driver +* +* Copyright (C) 2019 Texas Instruments +* Author: Bartosz Golaszewski <[email protected]>
include/clocksource/timer-davinci.h line 16
Quoted 10 lines
include/clocksource/timer-davinci.h new file +10
-0+*/ + +#ifndef __TIMER_DAVINCI_H__ +#define __TIMER_DAVINCI_H__ + +#include <linux/clk.h> +#include <linux/ioport.h> + +enum { + DAVINCI_TIMER_CLOCKEVENT_IRQ = 0,
Isn't = 0 implied, so can be omitted?
Quoted 4 lines
include/clocksource/timer-davinci.h new file +4
-0+DAVINCI_TIMER_CLOCKSOURCE_IRQ , + DAVINCI_TIMER_NUM_IRQS, +}; +
include/clocksource/timer-davinci.h line 30
Quoted 10 lines
include/clocksource/timer-davinci.h new file +10
-0+/** +* struct davinci_timer_cfg - davinci clocksource driver configuration struct +* @reg: register range resource +* @irq: clockevent and clocksource interrupt resources +* @cmp_off: if set - it specifies the compare register used for clockevent +* +* Note: if the compare register is specified, the driver will use the bottom +* clock half for both clocksource and clockevent and the compare register +* to generate event irqs. The user must supply the correct compare register +* interrupt number.
I'm a little confused by this comment. I think I get it, but it took me a while.
If cmp_off is 0, we don't use the compare register and therefore DAVINCI_TIMER_CLOCKEVENT_IRQ and DAVINCI_TIMER_CLOCKSOURCE_IRQ should be TINT12 and TINT34 (or vice versa?). If cmp_off is non-zero, then it should be one of the 8 (more or less depending on the SoC) and DAVINCI_TIMER_CLOCKEVENT_IRQ should be something like T12CMPINT0.
Quoted 15 lines
include/clocksource/timer-davinci.h new file +14
-0+* +* This is only used by da830 the DSP of which uses the top half. The timer +* driver still configures the top half to run in free-run mode. +*/ +struct davinci_timer_cfg { +struct resource reg; +struct resource irq[DAVINCI_TIMER_NUM_IRQS ]; +unsigned int cmp_off; +}; + +int __init davinci_timer_register (struct clk *clk, +const struct davinci_timer_cfg *data); + +#endif /* __TIMER_DAVINCI_H__ */
Re: [PATCH v2 02/12] clocksource: davinci-timer: new driver
In reply to Bartosz GolaszewskiQuoted 4 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Currently the clocksource and clockevent support for davinci platforms lives in mach-davinci. It hard-codes many things, used global variables,
s/used/uses/
Quoted 3 lines
implements functionalities unused by any platform and has code fragments scattered across many (often unrelated) files.
Re: [PATCH v2 02/12] clocksource: davinci-timer: new driver
In reply to Bartosz Golaszewskidrivers/clocksource/timer-davinci.c line 123
Quoted 11 lines · Bartosz Golaszewski
drivers/clocksource/timer-davinci.c new file +11
-0+static unsigned int davinci_timer_read (struct davinci_timer_data *timer, +unsigned int reg) +{ +return __raw_readl (timer->base + reg); +} + +static void davinci_timer_write (struct davinci_timer_data *timer, +unsigned int reg,unsigned int val) +{ +__raw_writel (val, timer->base + reg); +}
Since its a new driver, please use readl_relaxed() and writel_relaxed().
drivers/clocksource/timer-davinci.c line 273
Quoted 12 lines
drivers/clocksource/timer-davinci.c new file +12
-0+static void davinci_timer_init (void __iomem *base) +{ +/* Set clock to internal mode and disable it. */ +__raw_writel (0x0 , base +DAVINCI_TIMER_REG_TCR ); +/* +* Reset both 32-bit timers, set no prescaler for timer 34, set the +* timer to dual 32-bit unchained mode, unreset both 32-bit timers. +*/ +__raw_writel (DAVINCI_TIMER_TGCR_DEFAULT , base +DAVINCI_TIMER_REG_TGCR ); +/* Init both counters to zero. */ +__raw_writel (0x0 , base +DAVINCI_TIMER_REG_TIM12 ); +__raw_writel (0x0 , base +DAVINCI_TIMER_REG_TIM34 );
here too.
Quoted 5 lines
drivers/clocksource/timer-davinci.c new file +5
-0+} + +int __init davinci_timer_register (struct clk *clk, +const struct davinci_timer_cfg *timer_cfg) +{
[...]
Quoted 10 lines
drivers/clocksource/timer-davinci.c new file +10
-0+ clocksource =kzalloc (sizeof (*clocksource),GFP_KERNEL ); +if (!clocksource) { +pr_err ("%s: Error allocating memory for clocksource data\n" , + __func__); +return -ENOMEM ; + } + + clocksource->dev.name ="tim34" ; + clocksource->dev.rating =300 ; + clocksource->dev.read = davinci_timer_clksrc_read;
drivers/clocksource/timer-davinci.c line 367
Quoted 10 lines
drivers/clocksource/timer-davinci.c new file +10
-0+ clocksource->dev.mask =CLOCKSOURCE_MASK (DAVINCI_TIMER_CLKSRC_BITS ); + clocksource->dev.flags =CLOCK_SOURCE_IS_CONTINUOUS ; + clocksource->timer.set_period = davinci_timer_set_period_std; + clocksource->timer.mode =DAVINCI_TIMER_MODE_PERIODIC ; + clocksource->timer.base = base; + +if (timer_cfg->cmp_off) + clocksource->timer.regs = &davinci_timer_tim12_regs; +else + clocksource->timer.regs = &davinci_timer_tim34_regs;
We should set clocksource->dev.name based on cmp_off too, otherwise it will be confusing to have a device called "tim34" using tim12 registers.
Quoted 5 lines
+/**
+ * structdavinci_timer_cfg- davinci clocksource driver configuration struct+ * @reg: register range resource + * @irq: clockevent and clocksource interrupt resources + * @cmp_off: if set - it specifies the compare register used for clockevent
Quoted 8 lines
+ *
+ * Note: if the compare register is specified, the driver will use the bottom
+ * clock half for both clocksource and clockevent and the compare register
+ * to generate event irqs. The user must supply the correct compare register
+ * interrupt number.
+ *
+ * This is only used by da830 the DSP of which uses the top half. The timer
+ * driver still configures the top half to run in free-run mode.
This note helps. Thanks!
Regards,
Sekhar
PATCH v2 3/12 ARM: davinci: enable the clocksource driver for DT mode
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Switch all davinci boards using device tree to using the new clocksource driver: remove the previous OF_TIMER_DECLARE() from mach-davinci and select davinci-timer to be built for davinci_all_defconfig.
--- arch/arm/Kconfig | 1 + arch/arm/mach-davinci/time.c | 14 -------------- 2 files changed, 1 insertion(+), 14 deletions(-)
arch/arm/Kconfig +1 -0
@@ -586,6 +586,7 @@ config ARCH_DAVINCI select ARCH_HAS_HOLES_MEMORYMODEL select COMMON_CLK select CPU_ARM926T + select DAVINCI_TIMER select GENERIC_ALLOCATOR select GENERIC_CLOCKEVENTS select GENERIC_IRQ_CHIP
arch/arm/mach-davinci/time.c +0 -14
@@ -398,17 +398,3 @@ void __init davinci_timer_init(struct clk *timer_clk)for (i=0 ; i<ARRAY_SIZE (timers); i++)timer32_config (&timers[i]); } - -static int __init of_davinci_timer_init (struct device_node *np) -{ -struct clk *clk; - - clk =of_clk_get (np,0 ); -if (IS_ERR (clk)) -return PTR_ERR (clk); - -davinci_timer_init (clk); - -return 0 ; -} -TIMER_OF_DECLARE (davinci_timer,"ti,da830-timer" , of_davinci_timer_init);
Signature · 1 line
2.20.1
Re: [PATCH v2 03/12] ARM: davinci: enable the clocksource driver for DT mode
Quoted 6 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Switch all davinci boards using device tree to using the new clocksource driver: remove the previous
OF_TIMER_DECLARE()from mach-davinci and select davinci-timer to be built fordavinci_all_defconfig.
I don't see how this patch is related to davinci_all_defconfig. It looks like the new driver is just selected when ARCH_DAVINCI is selected in Kconfig.
Quoted 3 lines
Signed-off-by Bartosz Golaszewski <[email protected]>---
PATCH v2 4/12 ARM: davinci: WARN_ON() if clk_get() fails
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Currently the timer code checks if the clock pointer passed to it is good (!IS_ERR(clk)). The new clocksource driver expects the clock to be functional and doesn't perform any checks so emit a warning if clk_get() fails. Apply this to all davinci platforms.
--- arch/arm/mach-davinci/da830.c | 4 ++++ arch/arm/mach-davinci/da850.c | 4 ++++ arch/arm/mach-davinci/dm355.c | 4 ++++ arch/arm/mach-davinci/dm365.c | 4 ++++ arch/arm/mach-davinci/dm644x.c | 4 ++++ arch/arm/mach-davinci/dm646x.c | 4 ++++ 6 files changed, 24 insertions(+)
arch/arm/mach-davinci/da830.c +4 -0
@@ -834,6 +834,10 @@ void __init da830_init_time(void)da830_pll_init (NULL , pll,NULL ); clk =clk_get (NULL ,"timer0" ); +if (WARN_ON (IS_ERR (clk))) { +pr_err ("Unable to get the timer clock\n" ); +return ; + }davinci_timer_init (clk); }
arch/arm/mach-davinci/da850.c +4 -0
@@ -774,6 +774,10 @@ void __init da850_init_time(void)da850_pll0_init (NULL , pll0, cfgchip); clk =clk_get (NULL ,"timer0" ); +if (WARN_ON (IS_ERR (clk))) { +pr_err ("Unable to get the timer clock\n" ); +return ; + }davinci_timer_init (clk); }
arch/arm/mach-davinci/dm355.c +4 -0
@@ -745,6 +745,10 @@ void __init dm355_init_time(void)dm355_psc_init (NULL , psc); clk =clk_get (NULL ,"timer0" ); +if (WARN_ON (IS_ERR (clk))) { +pr_err ("Unable to get the timer clock\n" ); +return ; + }davinci_timer_init (clk); }
arch/arm/mach-davinci/dm365.c +4 -0
@@ -786,6 +786,10 @@ void __init dm365_init_time(void)dm365_psc_init (NULL , psc); clk =clk_get (NULL ,"timer0" ); +if (WARN_ON (IS_ERR (clk))) { +pr_err ("Unable to get the timer clock\n" ); +return ; + }davinci_timer_init (clk); }
arch/arm/mach-davinci/dm644x.c +4 -0
@@ -681,6 +681,10 @@ void __init dm644x_init_time(void)dm644x_psc_init (NULL , psc); clk =clk_get (NULL ,"timer0" ); +if (WARN_ON (IS_ERR (clk))) { +pr_err ("Unable to get the timer clock\n" ); +return ; + }davinci_timer_init (clk); }
arch/arm/mach-davinci/dm646x.c +4 -0
@@ -665,6 +665,10 @@ void __init dm646x_init_time(unsigned long ref_clk_rate,dm646x_psc_init (NULL , psc); clk =clk_get (NULL ,"timer0" ); +if (WARN_ON (IS_ERR (clk))) { +pr_err ("Unable to get the timer clock\n" ); +return ; + }davinci_timer_init (clk); }
Signature · 1 line
2.20.1
Re: [PATCH v2 04/12] ARM: davinci: WARN_ON() if clk_get() fails
Quoted 22 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Currently the timer code checks if the clock pointer passed to it is good (!IS_ERR(clk)). The new clocksource driver expects the clock to be functional and doesn't perform any checks so emit a warning if
clk_get()fails. Apply this to all davinci platforms.Signed-off-by Bartosz Golaszewski <[email protected]>--- arch/arm/mach-davinci/da830.c | 4 ++++ arch/arm/mach-davinci/da850.c | 4 ++++ arch/arm/mach-davinci/dm355.c | 4 ++++ arch/arm/mach-davinci/dm365.c | 4 ++++ arch/arm/mach-davinci/dm644x.c | 4 ++++ arch/arm/mach-davinci/dm646x.c | 4 ++++ 6 files changed, 24 insertions(+)arch/arm/mach-davinci/da830.c +0
-0
arch/arm/mach-davinci/da830.c void __init da830_init_time(void) · line 838
Quoted 5 lines
arch/arm/mach-davinci/da830.c +2
-0@@ -834,6 +834,10 @@ void __init da830_init_time(void)da830_pll_init (NULL , pll,NULL ); clk =clk_get (NULL ,"timer0" ); +if (WARN_ON (IS_ERR (clk))) { +pr_err ("Unable to get the timer clock\n" );
Do we really need a warning _and_ an error?
Quoted 5 lines
arch/arm/mach-davinci/da830.c +2
-0+return ; + }davinci_timer_init (clk); }
Re: [PATCH v2 04/12] ARM: davinci: WARN_ON() if clk_get() fails
wt., 5 lut 2019 o 03:18 David Lechner <[email protected]> napisał(a):
Quoted 26 lines · Bartosz Golaszewski
Quoted 24 lines
From: Bartosz Golaszewski <[email protected]>
Currently the timer code checks if the clock pointer passed to it is good (!IS_ERR(clk)). The new clocksource driver expects the clock to be functional and doesn't perform any checks so emit a warning if
clk_get()fails. Apply this to all davinci platforms.Signed-off-by Bartosz Golaszewski <[email protected]>--- arch/arm/mach-davinci/da830.c | 4 ++++ arch/arm/mach-davinci/da850.c | 4 ++++ arch/arm/mach-davinci/dm355.c | 4 ++++ arch/arm/mach-davinci/dm365.c | 4 ++++ arch/arm/mach-davinci/dm644x.c | 4 ++++ arch/arm/mach-davinci/dm646x.c | 4 ++++ 6 files changed, 24 insertions(+)arch/arm/mach-davinci/da830.c +0
-0@@ -834,6 +834,10 @@ void __init da830_init_time(void)da830_pll_init (NULL , pll,NULL );
Quoted 6 lines
arch/arm/mach-davinci/da830.c void __init da830_init_time(void) · line 838
Quoted 3 lines
arch/arm/mach-davinci/da830.c +2
-0clk =clk_get (NULL ,"timer0" ); +if (WARN_ON (IS_ERR (clk))) { +pr_err ("Unable to get the timer clock\n" );Do we really need a warning _and_ an error?
That will give us the reason AND point us to the right place in the code.
Bart
Quoted 5 lines
Quoted 5 lines
arch/arm/mach-davinci/da830.c +2
-0+return ; + }davinci_timer_init (clk); }
PATCH v2 5/12 ARM: davinci: da850: switch to using the clocksource driver
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
We now have a proper clocksource driver for davinci. Switch the platform to using it.
--- arch/arm/mach-davinci/da850.c | 56 ++++++++++++++--------------------- 1 file changed, 22 insertions(+), 34 deletions(-)
arch/arm/mach-davinci/da850.c +22 -34
@@ -34,7 +34,8 @@#include <mach/da8xx.h> #include <mach/irqs.h> #include <mach/pm.h> -#include <mach/time.h> + +#include <clocksource/timer-davinci.h> #include "mux.h"
@@ -436,38 +437,24 @@ static struct davinci_id da850_ids[] = { }, }; -static struct davinci_timer_instance da850_timer_instance[4 ] = { - { - .base =DA8XX_TIMER64P0_BASE , - .bottom_irq =IRQ_DA8XX_TINT12_0 , - .top_irq =IRQ_DA8XX_TINT34_0 , - }, - { - .base =DA8XX_TIMER64P1_BASE , - .bottom_irq =IRQ_DA8XX_TINT12_1 , - .top_irq =IRQ_DA8XX_TINT34_1 , - }, - { - .base =DA850_TIMER64P2_BASE , - .bottom_irq =IRQ_DA850_TINT12_2 , - .top_irq =IRQ_DA850_TINT34_2 , - }, - { - .base =DA850_TIMER64P3_BASE , - .bottom_irq =IRQ_DA850_TINT12_3 , - .top_irq =IRQ_DA850_TINT34_3 , +static const struct davinci_timer_cfg da850_timer_cfg = { + .reg = { + .start =DA8XX_TIMER64P0_BASE , + .end =DA8XX_TIMER64P0_BASE +SZ_4K , + .flags =IORESOURCE_MEM , }, -}; - -/* -* T0_BOT: Timer 0, bottom : Used for clock_event -* T0_TOP: Timer 0, top : Used for clocksource -* T1_BOT, T1_TOP: Timer 1, bottom & top: Used for watchdog timer -*/ -static struct davinci_timer_info da850_timer_info = { - .timers = da850_timer_instance, - .clockevent_id =T0_BOT , - .clocksource_id =T0_TOP , + .irq = { + { + .start =IRQ_DA8XX_TINT12_0 , + .end =IRQ_DA8XX_TINT12_0 , + .flags =IORESOURCE_IRQ , + }, + { + .start =IRQ_DA8XX_TINT34_0 , + .end =IRQ_DA8XX_TINT34_0 , + .flags =IORESOURCE_IRQ , + } + } };#ifdef CONFIG_CPU_FREQ
@@ -742,7 +729,6 @@ static const struct davinci_soc_info davinci_soc_info_da850 = { .intc_type = DAVINCI_INTC_TYPE_CP_INTC, .intc_irq_prios = da850_default_priorities, .intc_irq_num = DA850_N_CP_INTC_IRQ, - .timer_info = &da850_timer_info, .emac_pdata = &da8xx_emac_pdata, .sram_dma = DA8XX_SHARED_RAM_BASE, .sram_len = SZ_128K,
@@ -765,6 +751,7 @@ void __init da850_init_time(void)void __iomem *pll0;struct regmap *cfgchip;struct clk *clk; +int rv;clk_register_fixed_rate (NULL ,"ref_clk" ,NULL ,0 ,DA850_REF_FREQ );
@@ -779,7 +766,8 @@ void __init da850_init_time(void)return ; } -davinci_timer_init (clk); + rv =davinci_timer_register (clk, &da850_timer_cfg); +WARN (rv,"Unable to register the timer: %d\n" , rv); }static struct resource da850_pll1_resources[] = {
Signature · 1 line
2.20.1
Re: [PATCH v2 05/12] ARM: davinci: da850: switch to using the clocksource driver
Quoted 42 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
We now have a proper clocksource driver for davinci. Switch the platform to using it.
Signed-off-by Bartosz Golaszewski <[email protected]>--- arch/arm/mach-davinci/da850.c | 56 ++++++++++++++--------------------- 1 file changed, 22 insertions(+), 34 deletions(-)arch/arm/mach-davinci/da850.c +2
-15@@ -34,7 +34,8 @@#include <mach/da8xx.h> #include <mach/irqs.h> #include <mach/pm.h> -#include <mach/time.h> + +#include <clocksource/timer-davinci.h> #include "mux.h" @@ -436,38 +437,24 @@ static struct davinci_id da850_ids[] = { }, }; -static struct davinci_timer_instance da850_timer_instance[4 ] = { - { - .base =DA8XX_TIMER64P0_BASE , - .bottom_irq =IRQ_DA8XX_TINT12_0 , - .top_irq =IRQ_DA8XX_TINT34_0 , - }, - { - .base =DA8XX_TIMER64P1_BASE , - .bottom_irq =IRQ_DA8XX_TINT12_1 , - .top_irq =IRQ_DA8XX_TINT34_1 , - }, - { - .base =DA850_TIMER64P2_BASE , - .bottom_irq =IRQ_DA850_TINT12_2 ,
arch/arm/mach-davinci/da850.c static struct davinci_id da850_ids[] = { · line 443
Quoted 10 lines
arch/arm/mach-davinci/da850.c +4
-6- .top_irq = IRQ_DA850_TINT34_2, - }, - { - .base = DA850_TIMER64P3_BASE, - .bottom_irq = IRQ_DA850_TINT12_3, - .top_irq = IRQ_DA850_TINT34_3, +static const struct davinci_timer_cfg da850_timer_cfg = { + .reg = { + .start =DA8XX_TIMER64P0_BASE , + .end =DA8XX_TIMER64P0_BASE +SZ_4K ,
Missing minus one?
+ .end = DA8XX_TIMER64P0_BASE + SZ_4K - 1,
Quoted 5 lines
arch/arm/mach-davinci/da850.c +1
-3+ .flags = IORESOURCE_MEM, }, -}; - -/*
arch/arm/mach-davinci/da850.c static struct davinci_id da850_ids[] = { · line 447
Quoted 10 lines
arch/arm/mach-davinci/da850.c +2
-8-* T0_BOT: Timer 0, bottom : Used for clock_event -* T0_TOP: Timer 0, top : Used for clocksource -* T1_BOT, T1_TOP: Timer 1, bottom & top: Used for watchdog timer -*/ -static struct davinci_timer_info da850_timer_info = { - .timers = da850_timer_instance, - .clockevent_id =T0_BOT , - .clocksource_id =T0_TOP , + .irq = { + {
It would be nice to use the enum values from <clocksource/timer-davinci.h> here so we know which IRQ is which.
[DAVINCI_TIMER_CLOCKEVENT_IRQ] = {arch/arm/mach-davinci/da850.c static struct davinci_id da850_ids[] = { · line 449
Quoted 2 lines
arch/arm/mach-davinci/da850.c +2
-0+ .start = IRQ_DA8XX_TINT12_0, + .end = IRQ_DA8XX_TINT12_0,
Can .end be omitted since it is the same as .start?
arch/arm/mach-davinci/da850.c static struct davinci_id da850_ids[] = { · line 452
Quoted 3 lines
arch/arm/mach-davinci/da850.c +3
-0+ .flags = IORESOURCE_IRQ, + }, + {
[DAVINCI_TIMER_CLOCKSOURCE_IRQ] = {Quoted 17 lines
arch/arm/mach-davinci/da850.c +5
-1+ .start = IRQ_DA8XX_TINT34_0, + .end = IRQ_DA8XX_TINT34_0, + .flags = IORESOURCE_IRQ, + } + } };#ifdef CONFIG_CPU_FREQ @@ -742,7 +729,6 @@ static const struct davinci_soc_info davinci_soc_info_da850 = { .intc_type = DAVINCI_INTC_TYPE_CP_INTC, .intc_irq_prios = da850_default_priorities, .intc_irq_num = DA850_N_CP_INTC_IRQ, - .timer_info = &da850_timer_info, .emac_pdata = &da8xx_emac_pdata, .sram_dma = DA8XX_SHARED_RAM_BASE, .sram_len = SZ_128K,
arch/arm/mach-davinci/da850.c void __init da850_init_time(void) · line 754
Quoted 4 lines
arch/arm/mach-davinci/da850.c +1
-0@@ -765,6 +751,7 @@ void __init da850_init_time(void)void __iomem *pll0;struct regmap *cfgchip;struct clk *clk; +int rv;
bikeshed: int ret; and int err; are used way more often, so rv looks strange to me.
Quoted 14 lines
arch/arm/mach-davinci/da850.c +2
-1clk_register_fixed_rate (NULL ,"ref_clk" ,NULL ,0 ,DA850_REF_FREQ );@@ -779,7 +766,8 @@ void __init da850_init_time(void)return ; } -davinci_timer_init (clk); + rv =davinci_timer_register (clk, &da850_timer_cfg); +WARN (rv,"Unable to register the timer: %d\n" , rv); }static struct resource da850_pll1_resources[] = {
Re: [PATCH v2 05/12] ARM: davinci: da850: switch to using the clocksource driver
In reply to Bartosz Golaszewskiarch/arm/mach-davinci/da850.c static struct davinci_id da850_ids[] = { · line 443
Quoted 4 lines · Bartosz Golaszewski
arch/arm/mach-davinci/da850.c +4
-0+static const struct davinci_timer_cfg da850_timer_cfg = { + .reg = { + .start =DA8XX_TIMER64P0_BASE , + .end =DA8XX_TIMER64P0_BASE +SZ_4K ,
SZ_4K - 1
This should have prevented watchdog timer from getting registered.
Thanks,
Sekhar
Re: [PATCH v2 05/12] ARM: davinci: da850: switch to using the clocksource driver
pt., 8 lut 2019 o 13:06 Sekhar Nori <[email protected]> napisał(a):
Quoted 7 lines · Bartosz Golaszewski
arch/arm/mach-davinci/da850.c static struct davinci_id da850_ids[] = { · line 443
Quoted 4 lines
arch/arm/mach-davinci/da850.c +4
-0+static const struct davinci_timer_cfg da850_timer_cfg = { + .reg = { + .start =DA8XX_TIMER64P0_BASE , + .end =DA8XX_TIMER64P0_BASE +SZ_4K ,
Quoted 6 lines
SZ_4K- 1This should have prevented watchdog timer from getting registered.
Thanks,
Sekhar
My clocksource driver doesn't call request_region() so a subsequent devm_ioremap_resource() in the watchdog driver would still succeed. I now fixed both the missing call and the value here.
Bart
Re: [PATCH v2 05/12] ARM: davinci: da850: switch to using the clocksource driver
Quoted 10 lines · Bartosz Golaszewski
pt., 8 lut 2019 o 13:06 Sekhar Nori <[email protected]> napisał(a):
Quoted 9 lines · Bartosz Golaszewski
arch/arm/mach-davinci/da850.c static struct davinci_id da850_ids[] = { · line 443
Quoted 4 lines
arch/arm/mach-davinci/da850.c +4
-0+static const struct davinci_timer_cfg da850_timer_cfg = { + .reg = { + .start =DA8XX_TIMER64P0_BASE , + .end =DA8XX_TIMER64P0_BASE +SZ_4K ,
SZ_4K- 1
Quoted 8 lines
Quoted 4 lines
This should have prevented watchdog timer from getting registered.
Thanks,
SekharMy clocksource driver doesn't call
request_region()so a subsequentdevm_ioremap_resource()in the watchdog driver would still succeed. I now fixed both the missing call and the value here.
Ah, got it. Perhaps a call to request_region() should be added to catch issues?
Thanks,
Sekhar
Re: [PATCH v2 05/12] ARM: davinci: da850: switch to using the clocksource driver
pt., 8 lut 2019 o 13:37 Sekhar Nori <[email protected]> napisał(a):
Quoted 17 lines · Bartosz Golaszewski
Quoted 15 lines
pt., 8 lut 2019 o 13:06 Sekhar Nori <[email protected]> napisał(a):
Quoted 13 lines · Bartosz Golaszewski
arch/arm/mach-davinci/da850.c static struct davinci_id da850_ids[] = { · line 443
Quoted 4 lines
arch/arm/mach-davinci/da850.c +4
-0+static const struct davinci_timer_cfg da850_timer_cfg = { + .reg = { + .start =DA8XX_TIMER64P0_BASE , + .end =DA8XX_TIMER64P0_BASE +SZ_4K ,
SZ_4K- 1This should have prevented watchdog timer from getting registered.
Thanks,
Sekhar
Quoted 7 lines
Quoted 3 lines
My clocksource driver doesn't call
request_region()so a subsequentdevm_ioremap_resource()in the watchdog driver would still succeed. I now fixed both the missing call and the value here.Ah, got it. Perhaps a call to
request_region()should be added to catch issues?
Yes, that's what I meant by "fixing the missing call". It will be there in v3. Actually the kernel seems to be missing a non-managed ioremap_resource() helper. Maybe I should add it as well.
Bart
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 6/12 ARM: davinci: da830: switch to using the clocksource driver
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
We now have a proper clocksource driver for davinci. Switch the platform to using it.
--- arch/arm/mach-davinci/da830.c | 49 ++++++++++++++++------------------- 1 file changed, 22 insertions(+), 27 deletions(-)
arch/arm/mach-davinci/da830.c +22 -27
@@ -20,7 +20,8 @@#include <mach/cputype.h> #include <mach/da8xx.h> #include <mach/irqs.h> -#include <mach/time.h> + +#include <clocksource/timer-davinci.h> #include "mux.h"
@@ -769,32 +770,25 @@ int __init da830_register_gpio(void)return da8xx_register_gpio (&da830_gpio_platform_data); } -static struct davinci_timer_instance da830_timer_instance[2 ] = { - { - .base =DA8XX_TIMER64P0_BASE , - .bottom_irq =IRQ_DA8XX_TINT12_0 , - .top_irq =IRQ_DA8XX_TINT34_0 , - .cmp_off =DA830_CMP12_0 , - .cmp_irq =IRQ_DA830_T12CMPINT0_0 , +static const struct davinci_timer_cfg da830_timer_cfg = { + .reg = { + .start =DA8XX_TIMER64P0_BASE , + .end =DA8XX_TIMER64P0_BASE +SZ_4K , + .flags =IORESOURCE_MEM , }, - { - .base =DA8XX_TIMER64P1_BASE , - .bottom_irq =IRQ_DA8XX_TINT12_1 , - .top_irq =IRQ_DA8XX_TINT34_1 , - .cmp_off =DA830_CMP12_0 , - .cmp_irq =IRQ_DA830_T12CMPINT0_1 , + .irq = { + { + .start =IRQ_DA830_T12CMPINT0_0 , + .end =IRQ_DA830_T12CMPINT0_0 , + .flags =IORESOURCE_IRQ , + }, + { + .start =IRQ_DA8XX_TINT12_0 , + .end =IRQ_DA8XX_TINT12_0 , + .flags =IORESOURCE_IRQ , + }, }, -}; - -/* -* T0_BOT: Timer 0, bottom : Used for clock_event & clocksource -* T0_TOP: Timer 0, top : Used by DSP -* T1_BOT, T1_TOP: Timer 1, bottom & top: Used for watchdog timer -*/ -static struct davinci_timer_info da830_timer_info = { - .timers = da830_timer_instance, - .clockevent_id =T0_BOT , - .clocksource_id =T0_BOT , + .cmp_off =DA830_CMP12_0 , };static const struct davinci_soc_info davinci_soc_info_da830 = {
@@ -810,7 +804,6 @@ static const struct davinci_soc_info davinci_soc_info_da830 = { .intc_type = DAVINCI_INTC_TYPE_CP_INTC, .intc_irq_prios = da830_default_priorities, .intc_irq_num = DA830_N_CP_INTC_IRQ, - .timer_info = &da830_timer_info, .emac_pdata = &da8xx_emac_pdata, };
@@ -826,6 +819,7 @@ void __init da830_init_time(void) {void __iomem *pll;struct clk *clk; +int rv;clk_register_fixed_rate (NULL ,"ref_clk" ,NULL ,0 ,DA830_REF_FREQ );
@@ -839,7 +833,8 @@ void __init da830_init_time(void)return ; } -davinci_timer_init (clk); + rv =davinci_timer_register (clk, &da830_timer_cfg); +WARN (rv,"Unable to register the timer: %d\n" , rv); }static struct resource da830_psc0_resources[] = {
Signature · 1 line
2.20.1
Re: [PATCH v2 06/12] ARM: davinci: da830: switch to using the clocksource driver
Quoted 7 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
We now have a proper clocksource driver for davinci. Switch the platform to using it.
Signed-off-by Bartosz Golaszewski <[email protected]>---
same comments as da850 patch
Re: [PATCH v2 06/12] ARM: davinci: da830: switch to using the clocksource driver
In reply to Bartosz GolaszewskiQuoted 4 lines · Bartosz Golaszewski
-/*
- *T0_BOT: Timer 0, bottom : Used forclock_event& clocksource
- *T0_TOP: Timer 0, top : Used by DSP
- *T1_BOT,T1_TOP: Timer 1, bottom & top: Used for watchdog timer
Aren't we changing this usage model for DA830 now, leaving no clock for use by DSP? It seems to me that after these patches clockevent is always T0_BOT and clocksource is always T0_TOP.
Thanks,
Sekhar
Re: [PATCH v2 06/12] ARM: davinci: da830: switch to using the clocksource driver
pt., 8 lut 2019 o 13:23 Sekhar Nori <[email protected]> napisał(a):
Quoted 7 lines · Bartosz Golaszewski
Quoted 4 lines
-/*
- *T0_BOT: Timer 0, bottom : Used forclock_event& clocksource
- *T0_TOP: Timer 0, top : Used by DSP
- *T1_BOT,T1_TOP: Timer 1, bottom & top: Used for watchdog timer
Quoted 6 lines
Aren't we changing this usage model for DA830 now, leaving no clock for use by DSP? It seems to me that after these patches clockevent is always
T0_BOTand clocksource is alwaysT0_TOP.Thanks,
Sekhar
No it's still the same. I added some comments to the driver's header in v2.
Bart
PATCH v2 7/12 ARM: davinci: move timer definitions to davinci.h
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Boards from the dm* family rely on register offset definitions from arch/arm/mach-davinci/include/mach/time.h. We'll be removing this file soon, so move the required defines to davinci.h where the rest of such constants live.
--- arch/arm/mach-davinci/davinci.h | 3 +++ arch/arm/mach-davinci/include/mach/time.h | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-)
arch/arm/mach-davinci/davinci.h +3 -0
@@ -60,6 +60,9 @@ void davinci_map_sysmod(void);#define DAVINCI_GPIO_BASE 0x01C67000int davinci_gpio_register (struct resource *res,int size,void *pdata); +#define DAVINCI_TIMER0_BASE (IO_PHYS + 0x21400) +#define DAVINCI_WDOG_BASE (IO_PHYS + 0x21C00) +/* DM355 base addresses */ #define DM355_ASYNC_EMIF_CONTROL_BASE 0x01e10000#define DM355_ASYNC_EMIF_DATA_CE0_BASE 0x02000000
arch/arm/mach-davinci/include/mach/time.h +0 -2
@@ -11,9 +11,7 @@#ifndef __ARCH_ARM_MACH_DAVINCI_TIME_H#define __ARCH_ARM_MACH_DAVINCI_TIME_H -#define DAVINCI_TIMER0_BASE (IO_PHYS + 0x21400)#define DAVINCI_TIMER1_BASE (IO_PHYS + 0x21800) -#define DAVINCI_WDOG_BASE (IO_PHYS + 0x21C00)enum { T0_BOT,
Signature · 1 line
2.20.1
PATCH v2 8/12 ARM: davinci: dm355: switch to using the clocksource driver
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
We now have a proper clocksource driver for davinci. Switch the platform to using it.
--- arch/arm/mach-davinci/dm355.c | 36 ++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-)
arch/arm/mach-davinci/dm355.c +23 -13
@@ -29,7 +29,8 @@#include <mach/irqs.h> #include <mach/mux.h> #include <mach/serial.h> -#include <mach/time.h> + +#include <clocksource/timer-davinci.h> #include "asp.h" #include "davinci.h"
@@ -617,16 +618,24 @@ static struct davinci_id dm355_ids[] = { }, }; -/* -* T0_BOT: Timer 0, bottom: clockevent source for hrtimers -* T0_TOP: Timer 0, top : clocksource for generic timekeeping -* T1_BOT: Timer 1, bottom: (used by DSP in TI DSPLink code) -* T1_TOP: Timer 1, top : <unused> -*/ -static struct davinci_timer_info dm355_timer_info = { - .timers = davinci_timer_instance, - .clockevent_id =T0_BOT , - .clocksource_id =T0_TOP , +static const struct davinci_timer_cfg dm355_timer_cfg = { + .reg = { + .start =DAVINCI_TIMER0_BASE , + .end =DAVINCI_TIMER0_BASE +SZ_4K , + .flags =IORESOURCE_MEM , + }, + .irq = { + { + .start =IRQ_TINT0_TINT12 , + .end =IRQ_TINT0_TINT12 , + .flags =IORESOURCE_IRQ , + }, + { + .start =IRQ_TINT0_TINT34 , + .end =IRQ_TINT0_TINT34 , + .flags =IORESOURCE_IRQ , + } + } };static struct plat_serial8250_port dm355_serial0_platform_data[] = {
@@ -708,7 +717,6 @@ static const struct davinci_soc_info davinci_soc_info_dm355 = { .intc_type = DAVINCI_INTC_TYPE_AINTC, .intc_irq_prios = dm355_default_priorities, .intc_irq_num = DAVINCI_N_AINTC_IRQ, - .timer_info = &dm355_timer_info, .sram_dma = 0x00010000, .sram_len = SZ_32K, };
@@ -735,6 +743,7 @@ void __init dm355_init_time(void) {void __iomem *pll1, *psc;struct clk *clk; +int rv;clk_register_fixed_rate (NULL ,"ref_clk" ,NULL ,0 ,DM355_REF_FREQ );
@@ -750,7 +759,8 @@ void __init dm355_init_time(void)return ; } -davinci_timer_init (clk); + rv =davinci_timer_register (clk, &dm355_timer_cfg); +WARN (rv,"Unable to register the timer: %d\n" , rv); }static struct resource dm355_pll2_resources[] = {
Signature · 1 line
2.20.1
Re: [PATCH v2 08/12] ARM: davinci: dm355: switch to using the clocksource driver
Quoted 5 lines · Bartosz Golaszewski
-/*
- * T0_BOT: Timer 0, bottom: clockevent source for hrtimers - * T0_TOP: Timer 0, top : clocksource for generic timekeeping - * T1_BOT: Timer 1, bottom: (used by DSP in TI DSPLink code) - * T1_TOP: Timer 1, top : <unused>
Documenting this timer division is important because DSP usage and unused timers are not immediately clear from kernel code alone. Can you retain these comments in all patches of this series (I guess T0_BOT etc. terminology is changed, so feel free to adjust the comment text).
arch/arm/mach-davinci/dm355.c static struct davinci_id dm355_ids[] = { · line 624
Quoted 9 lines
- */
arch/arm/mach-davinci/dm355.c +4
-4-static struct davinci_timer_info dm355_timer_info = { - .timers = davinci_timer_instance, - .clockevent_id =T0_BOT , - .clocksource_id =T0_TOP , +static const struct davinci_timer_cfg dm355_timer_cfg = { + .reg = { + .start =DAVINCI_TIMER0_BASE , + .end =DAVINCI_TIMER0_BASE +SZ_4K ,
Here and other places too, SZ_4K - 1
Thanks,
Sekhar
Re: [PATCH v2 08/12] ARM: davinci: dm355: switch to using the clocksource driver
pt., 8 lut 2019 o 13:34 Sekhar Nori <[email protected]> napisał(a):
Quoted 25 lines · Bartosz Golaszewski
Quoted 5 lines
-/*
- * T0_BOT: Timer 0, bottom: clockevent source for hrtimers - * T0_TOP: Timer 0, top : clocksource for generic timekeeping - * T1_BOT: Timer 1, bottom: (used by DSP in TI DSPLink code) - * T1_TOP: Timer 1, top : <unused>Documenting this timer division is important because DSP usage and unused timers are not immediately clear from kernel code alone. Can you retain these comments in all patches of this series (I guess
T0_BOTetc. terminology is changed, so feel free to adjust the comment text).arch/arm/mach-davinci/dm355.c static struct davinci_id dm355_ids[] = { · line 624
Quoted 9 lines
- */
arch/arm/mach-davinci/dm355.c +4
-4-static struct davinci_timer_info dm355_timer_info = { - .timers = davinci_timer_instance, - .clockevent_id =T0_BOT , - .clocksource_id =T0_TOP , +static const struct davinci_timer_cfg dm355_timer_cfg = { + .reg = { + .start =DAVINCI_TIMER0_BASE , + .end =DAVINCI_TIMER0_BASE +SZ_4K ,Here and other places too,
SZ_4K- 1
Quoted 2 lines
Thanks,
Sekhar
I used appropriate DEFINE_RES_x() macros instead.
Bart
PATCH v2 9/12 ARM: davinci: dm365: switch to using the clocksource driver
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
We now have a proper clocksource driver for davinci. Switch the platform to using it.
--- arch/arm/mach-davinci/dm365.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-)
arch/arm/mach-davinci/dm365.c +23 -7
@@ -34,7 +34,8 @@#include <mach/irqs.h> #include <mach/mux.h> #include <mach/serial.h> -#include <mach/time.h> + +#include <clocksource/timer-davinci.h> #include "asp.h" #include "davinci.h"
@@ -658,10 +659,24 @@ static struct davinci_id dm365_ids[] = { }, }; -static struct davinci_timer_info dm365_timer_info = { - .timers = davinci_timer_instance, - .clockevent_id =T0_BOT , - .clocksource_id =T0_TOP , +static const struct davinci_timer_cfg dm365_timer_cfg = { + .reg = { + .start =DAVINCI_TIMER0_BASE , + .end =DAVINCI_TIMER0_BASE +SZ_4K , + .flags =IORESOURCE_MEM , + }, + .irq = { + { + .start =IRQ_TINT0_TINT12 , + .end =IRQ_TINT0_TINT12 , + .flags =IORESOURCE_IRQ , + }, + { + .start =IRQ_TINT0_TINT34 , + .end =IRQ_TINT0_TINT34 , + .flags =IORESOURCE_IRQ , + } + } };#define DM365_UART1_BASE (IO_PHYS + 0x106000)
@@ -725,7 +740,6 @@ static const struct davinci_soc_info davinci_soc_info_dm365 = { .intc_type = DAVINCI_INTC_TYPE_AINTC, .intc_irq_prios = dm365_default_priorities, .intc_irq_num = DAVINCI_N_AINTC_IRQ, - .timer_info = &dm365_timer_info, .emac_pdata = &dm365_emac_pdata, .sram_dma = 0x00010000, .sram_len = SZ_32K,
@@ -773,6 +787,7 @@ void __init dm365_init_time(void) {void __iomem *pll1, *pll2, *psc;struct clk *clk; +int rv;clk_register_fixed_rate (NULL ,"ref_clk" ,NULL ,0 ,DM365_REF_FREQ );
@@ -791,7 +806,8 @@ void __init dm365_init_time(void)return ; } -davinci_timer_init (clk); + rv =davinci_timer_register (clk, &dm365_timer_cfg); +WARN (rv,"Unable to register the timer: %d\n" , rv); }void __init dm365_register_clocks (void )
Signature · 1 line
2.20.1
PATCH v2 10/12 ARM: davinci: dm644x: switch to using the clocksource driver
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
We now have a proper clocksource driver for davinci. Switch the platform to using it.
--- arch/arm/mach-davinci/dm644x.c | 36 ++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 13 deletions(-)
arch/arm/mach-davinci/dm644x.c +23 -13
@@ -26,7 +26,8 @@#include <mach/irqs.h> #include <mach/mux.h> #include <mach/serial.h> -#include <mach/time.h> + +#include <clocksource/timer-davinci.h> #include "asp.h" #include "davinci.h"
@@ -558,16 +559,24 @@ static struct davinci_id dm644x_ids[] = { }, }; -/* -* T0_BOT: Timer 0, bottom: clockevent source for hrtimers -* T0_TOP: Timer 0, top : clocksource for generic timekeeping -* T1_BOT: Timer 1, bottom: (used by DSP in TI DSPLink code) -* T1_TOP: Timer 1, top : <unused> -*/ -static struct davinci_timer_info dm644x_timer_info = { - .timers = davinci_timer_instance, - .clockevent_id =T0_BOT , - .clocksource_id =T0_TOP , +static const struct davinci_timer_cfg dm644x_timer_cfg = { + .reg = { + .start =DAVINCI_TIMER0_BASE , + .end =DAVINCI_TIMER0_BASE +SZ_4K , + .flags =IORESOURCE_MEM , + }, + .irq = { + { + .start =IRQ_TINT0_TINT12 , + .end =IRQ_TINT0_TINT12 , + .flags =IORESOURCE_IRQ , + }, + { + .start =IRQ_TINT0_TINT34 , + .end =IRQ_TINT0_TINT34 , + .flags =IORESOURCE_IRQ , + } + } };static struct plat_serial8250_port dm644x_serial0_platform_data[] = {
@@ -649,7 +658,6 @@ static const struct davinci_soc_info davinci_soc_info_dm644x = { .intc_type = DAVINCI_INTC_TYPE_AINTC, .intc_irq_prios = dm644x_default_priorities, .intc_irq_num = DAVINCI_N_AINTC_IRQ, - .timer_info = &dm644x_timer_info, .emac_pdata = &dm644x_emac_pdata, .sram_dma = 0x00008000, .sram_len = SZ_16K,
@@ -671,6 +679,7 @@ void __init dm644x_init_time(void) {void __iomem *pll1, *psc;struct clk *clk; +int rv;clk_register_fixed_rate (NULL ,"ref_clk" ,NULL ,0 ,DM644X_REF_FREQ );
@@ -686,7 +695,8 @@ void __init dm644x_init_time(void)return ; } -davinci_timer_init (clk); + rv =davinci_timer_register (clk, &dm644x_timer_cfg); +WARN (rv,"Unable to register the timer: %d\n" , rv); }static struct resource dm644x_pll2_resources[] = {
Signature · 1 line
2.20.1
PATCH v2 11/12 ARM: davinci: dm646x: switch to using the clocksource driver
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
We now have a proper clocksource driver for davinci. Switch the platform to using it.
--- arch/arm/mach-davinci/dm646x.c | 36 ++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 13 deletions(-)
arch/arm/mach-davinci/dm646x.c +23 -13
@@ -27,7 +27,8 @@#include <mach/irqs.h> #include <mach/mux.h> #include <mach/serial.h> -#include <mach/time.h> + +#include <clocksource/timer-davinci.h> #include "asp.h" #include "davinci.h"
@@ -498,16 +499,24 @@ static struct davinci_id dm646x_ids[] = { }, }; -/* -* T0_BOT: Timer 0, bottom: clockevent source for hrtimers -* T0_TOP: Timer 0, top : clocksource for generic timekeeping -* T1_BOT: Timer 1, bottom: (used by DSP in TI DSPLink code) -* T1_TOP: Timer 1, top : <unused> -*/ -static struct davinci_timer_info dm646x_timer_info = { - .timers = davinci_timer_instance, - .clockevent_id =T0_BOT , - .clocksource_id =T0_TOP , +static const struct davinci_timer_cfg dm646x_timer_cfg = { + .reg = { + .start =DAVINCI_TIMER0_BASE , + .end =DAVINCI_TIMER0_BASE +SZ_4K , + .flags =IORESOURCE_MEM , + }, + .irq = { + { + .start =IRQ_TINT0_TINT12 , + .end =IRQ_TINT0_TINT12 , + .flags =IORESOURCE_IRQ , + }, + { + .start =IRQ_TINT0_TINT34 , + .end =IRQ_TINT0_TINT34 , + .flags =IORESOURCE_IRQ , + } + } };static struct plat_serial8250_port dm646x_serial0_platform_data[] = {
@@ -589,7 +598,6 @@ static const struct davinci_soc_info davinci_soc_info_dm646x = { .intc_type = DAVINCI_INTC_TYPE_AINTC, .intc_irq_prios = dm646x_default_priorities, .intc_irq_num = DAVINCI_N_AINTC_IRQ, - .timer_info = &dm646x_timer_info, .emac_pdata = &dm646x_emac_pdata, .sram_dma = 0x10010000, .sram_len = SZ_32K,
@@ -654,6 +662,7 @@ void __init dm646x_init_time(unsigned long ref_clk_rate, {void __iomem *pll1, *psc;struct clk *clk; +int rv;clk_register_fixed_rate (NULL ,"ref_clk" ,NULL ,0 , ref_clk_rate);clk_register_fixed_rate (NULL ,"aux_clkin" ,NULL ,0 , aux_clkin_rate);
@@ -670,7 +679,8 @@ void __init dm646x_init_time(unsigned long ref_clk_rate,return ; } -davinci_timer_init (clk); + rv =davinci_timer_register (clk, &dm646x_timer_cfg); +WARN (rv,"Unable to register the timer: %d\n" , rv); }static struct resource dm646x_pll2_resources[] = {
Signature · 1 line
2.20.1
PATCH v2 12/12 ARM: davinci: remove legacy timer support
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
All platforms have now been switched to the new clocksource driver. Remove the old code and various no longer needed bits and pieces.
--- arch/arm/mach-davinci/Makefile | 3 +- arch/arm/mach-davinci/devices-da8xx.c | 1 - arch/arm/mach-davinci/devices.c | 19 - arch/arm/mach-davinci/include/mach/common.h | 17 - arch/arm/mach-davinci/include/mach/time.h | 33 -- arch/arm/mach-davinci/time.c | 400 -------------------- 6 files changed, 1 insertion(+), 472 deletions(-) delete mode 100644 arch/arm/mach-davinci/include/mach/time.h delete mode 100644 arch/arm/mach-davinci/time.c
arch/arm/mach-davinci/Makefile +1 -2
@@ -5,8 +5,7 @@ # # Common objects -obj-y := time.o serial.o usb.o \ - common.o sram.o +obj-y := serial.o usb.o common.o sram.o obj-$(CONFIG_DAVINCI_MUX) += mux.o
arch/arm/mach-davinci/devices-da8xx.c +0 -1
@@ -24,7 +24,6 @@#include <mach/common.h> #include <mach/cputype.h> #include <mach/da8xx.h> -#include <mach/time.h> #include "asp.h" #include "cpuidle.h"
arch/arm/mach-davinci/devices.c +0 -19
@@ -21,7 +21,6 @@#include <mach/cputype.h> #include <mach/mux.h> #include <linux/platform_data/mmc-davinci.h> -#include <mach/time.h> #include <linux/platform_data/edma.h>
@@ -305,21 +304,3 @@ int davinci_gpio_register(struct resource *res, int size, void *pdata) davinci_gpio_device.dev.platform_data = pdata;return platform_device_register (&davinci_gpio_device); } - -/*-------------------------------------------------------------------------*/ - -/*-------------------------------------------------------------------------*/ - -struct davinci_timer_instance davinci_timer_instance[2 ] = { - { - .base =DAVINCI_TIMER0_BASE , - .bottom_irq =IRQ_TINT0_TINT12 , - .top_irq =IRQ_TINT0_TINT34 , - }, - { - .base =DAVINCI_TIMER1_BASE , - .bottom_irq =IRQ_TINT1_TINT12 , - .top_irq =IRQ_TINT1_TINT34 , - }, -}; -
arch/arm/mach-davinci/include/mach/common.h +0 -17
@@ -17,26 +17,10 @@#include <linux/types.h> #include <linux/reboot.h> -void davinci_timer_init (struct clk *clk); -extern void davinci_irq_init (void );extern void __iomem *davinci_intc_base;extern int davinci_intc_type; -struct davinci_timer_instance { -u32 base; -u32 bottom_irq; -u32 top_irq; -unsigned long cmp_off; -unsigned int cmp_irq; -}; - -struct davinci_timer_info { -struct davinci_timer_instance *timers; -unsigned int clockevent_id; -unsigned int clocksource_id; -}; -struct davinci_gpio_controller ;/*
@@ -62,7 +46,6 @@ struct davinci_soc_info {u8 *intc_irq_prios;unsigned long intc_irq_num;u32 *intc_host_map; -struct davinci_timer_info *timer_info;int gpio_type;u32 gpio_base;unsigned gpio_num;
arch/arm/mach-davinci/include/mach/time.h deleted +0 -33
@@ -1,33 +0,0 @@ -/* -* Local header file for DaVinci time code. -* -* Author: Kevin Hilman, MontaVista Software, Inc. <[email protected]> -* -* 2007 (c) MontaVista Software, Inc. This file is licensed under -* the terms of the GNU General Public License version 2. This program -* is licensed "as is" without any warranty of any kind, whether express -* or implied. -*/ -#ifndef __ARCH_ARM_MACH_DAVINCI_TIME_H -#define __ARCH_ARM_MACH_DAVINCI_TIME_H - -#define DAVINCI_TIMER1_BASE (IO_PHYS + 0x21800) - -enum { -T0_BOT , -T0_TOP , -T1_BOT , -T1_TOP , -NUM_TIMERS -}; - -#define IS_TIMER1 (id) (id & 0x2) -#define IS_TIMER0 (id) (!IS_TIMER1(id)) -#define IS_TIMER_TOP (id) ((id & 0x1)) -#define IS_TIMER_BOT (id) (!IS_TIMER_TOP(id)) - -#define ID_TO_TIMER (id) (IS_TIMER1(id) != 0) - -extern struct davinci_timer_instance davinci_timer_instance[]; - -#endif /* __ARCH_ARM_MACH_DAVINCI_TIME_H */
arch/arm/mach-davinci/time.c deleted +0 -400
400 lines not on this page · raw message
Signature · 1 line
2.20.1