From: Bartosz Golaszewski <redacted>
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
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Bartosz Golaszewski <redacted>
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.
Fixes: 3652e2741f42 ("ARM: dts: da850: Add clocks")
Cc: stable@vger.kernel.org
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/boot/dts/da850.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: David Lechner <david@lechnology.com> Date: 2019-02-05 01:18:19
On 2/4/19 11:17 AM, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski <redacted>
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.
Fixes: 3652e2741f42 ("ARM: dts: da850: Add clocks")
Cc: stable@vger.kernel.org
Signed-off-by: Bartosz Golaszewski <redacted>
---
From: Sekhar Nori <hidden> Date: 2019-02-08 13:10:47
On 05/02/19 6:48 AM, David Lechner wrote:
On 2/4/19 11:17 AM, Bartosz Golaszewski wrote:
quoted
From: Bartosz Golaszewski <redacted>
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.
Fixes: 3652e2741f42 ("ARM: dts: da850: Add clocks")
Cc: stable@vger.kernel.org
Signed-off-by: Bartosz Golaszewski <redacted>
---
From: Bartosz Golaszewski <redacted>
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.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/Kconfig | 1 +
arch/arm/mach-davinci/time.c | 14 --------------
2 files changed, 1 insertion(+), 14 deletions(-)
From: David Lechner <david@lechnology.com> Date: 2019-02-05 02:16:31
On 2/4/19 11:17 AM, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski <redacted>
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.
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.
From: Bartosz Golaszewski <redacted>
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 <redacted>
---
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
@@ -0,0 +1,431 @@+// SPDX-License-Identifier: GPL-2.0+//+// TI DaVinci clocksource driver+//+// Copyright (C) 2019 Texas Instruments+// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>+// (with some parts adopted from code by Kevin Hilman <khilman@baylibre.com>)++#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)+#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)++#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,+DAVINCI_TIMER_MODE_PERIODIC,+};++structdavinci_timer_data;++typedefvoid(*davinci_timer_set_period_func)(structdavinci_timer_data*,+unsignedintperiod);++structdavinci_timer_regs{+unsignedinttim_off;+unsignedintprd_off;+unsignedintenamode_shift;+};++structdavinci_timer_data{+void__iomem*base;+conststructdavinci_timer_regs*regs;+unsignedintmode;+davinci_timer_set_period_funcset_period;+unsignedintcmp_off;+};++structdavinci_timer_clockevent{+structclock_event_devicedev;+unsignedinttick_rate;+structdavinci_timer_datatimer;+};++structdavinci_timer_clocksource{+structclocksourcedev;+structdavinci_timer_datatimer;+};++staticconststructdavinci_timer_regsdavinci_timer_tim12_regs={+.tim_off=DAVINCI_TIMER_REG_TIM12,+.prd_off=DAVINCI_TIMER_REG_PRD12,+.enamode_shift=DAVINCI_TIMER_ENAMODE_SHIFT_TIM12,+};++staticconststructdavinci_timer_regsdavinci_timer_tim34_regs={+.tim_off=DAVINCI_TIMER_REG_TIM34,+.prd_off=DAVINCI_TIMER_REG_PRD34,+.enamode_shift=DAVINCI_TIMER_ENAMODE_SHIFT_TIM34,+};++/* Must be global for davinci_timer_read_sched_clock(). */+staticstructdavinci_timer_data*davinci_timer_clksrc_timer;++staticstructdavinci_timer_clockevent*+to_davinci_timer_clockevent(structclock_event_device*clockevent)+{+returncontainer_of(clockevent,structdavinci_timer_clockevent,dev);+}++staticstructdavinci_timer_clocksource*+to_davinci_timer_clocksource(structclocksource*clocksource)+{+returncontainer_of(clocksource,structdavinci_timer_clocksource,dev);+}++staticunsignedintdavinci_timer_read(structdavinci_timer_data*timer,+unsignedintreg)+{+return__raw_readl(timer->base+reg);+}++staticvoiddavinci_timer_write(structdavinci_timer_data*timer,+unsignedintreg,unsignedintval)+{+__raw_writel(val,timer->base+reg);+}++staticvoiddavinci_timer_update(structdavinci_timer_data*timer,+unsignedintreg,unsignedintmask,+unsignedintval)+{+unsignedintnew,orig=davinci_timer_read(timer,reg);++new=orig&~mask;+new|=val&mask;++davinci_timer_write(timer,reg,new);+}++staticvoiddavinci_timer_set_period(structdavinci_timer_data*timer,+unsignedintperiod)+{+timer->set_period(timer,period);+}++staticvoiddavinci_timer_set_period_std(structdavinci_timer_data*timer,+unsignedintperiod)+{+conststructdavinci_timer_regs*regs=timer->regs;+unsignedintenamode;++enamode=davinci_timer_read(timer,DAVINCI_TIMER_REG_TCR);++davinci_timer_update(timer,DAVINCI_TIMER_REG_TCR,+DAVINCI_TIMER_ENAMODE_MASK<<regs->enamode_shift,+DAVINCI_TIMER_ENAMODE_DISABLED<<regs->enamode_shift);++davinci_timer_write(timer,regs->tim_off,0x0);+davinci_timer_write(timer,regs->prd_off,period);++if(timer->mode==DAVINCI_TIMER_MODE_ONESHOT)+enamode=DAVINCI_TIMER_ENAMODE_ONESHOT;+elseif(timer->mode==DAVINCI_TIMER_MODE_PERIODIC)+enamode=DAVINCI_TIMER_ENAMODE_PERIODIC;++davinci_timer_update(timer,DAVINCI_TIMER_REG_TCR,+DAVINCI_TIMER_ENAMODE_MASK<<regs->enamode_shift,+enamode<<regs->enamode_shift);+}++staticvoiddavinci_timer_set_period_cmp(structdavinci_timer_data*timer,+unsignedintperiod)+{+conststructdavinci_timer_regs*regs=timer->regs;+unsignedintcurr_time;++curr_time=davinci_timer_read(timer,regs->tim_off);+davinci_timer_write(timer,timer->cmp_off,curr_time+period);+}++staticirqreturn_tdavinci_timer_irq_timer(intirq,void*data)+{+structdavinci_timer_clockevent*clockevent=data;++clockevent->dev.event_handler(&clockevent->dev);++returnIRQ_HANDLED;+}++staticirqreturn_tdavinci_timer_irq_freerun(intirq,void*data)+{+returnIRQ_HANDLED;+}++staticu64notracedavinci_timer_read_sched_clock(void)+{+structdavinci_timer_data*timer;+unsignedintval;++timer=davinci_timer_clksrc_timer;+val=davinci_timer_read(timer,timer->regs->tim_off);++returnval;+}++staticu64davinci_timer_clksrc_read(structclocksource*dev)+{+structdavinci_timer_clocksource*clocksource;+conststructdavinci_timer_regs*regs;+unsignedintval;++clocksource=to_davinci_timer_clocksource(dev);+regs=clocksource->timer.regs;++val=davinci_timer_read(&clocksource->timer,regs->tim_off);++returnval;+}++staticintdavinci_timer_set_next_event(unsignedlongcycles,+structclock_event_device*dev)+{+structdavinci_timer_clockevent*clockevent;++clockevent=to_davinci_timer_clockevent(dev);+davinci_timer_set_period(&clockevent->timer,cycles);++return0;+}++staticintdavinci_timer_set_state_shutdown(structclock_event_device*dev)+{+structdavinci_timer_clockevent*clockevent;++clockevent=to_davinci_timer_clockevent(dev);+clockevent->timer.mode=DAVINCI_TIMER_MODE_DISABLED;++return0;+}++staticintdavinci_timer_set_state_periodic(structclock_event_device*dev)+{+structdavinci_timer_clockevent*clockevent;+unsignedintperiod;++clockevent=to_davinci_timer_clockevent(dev);+period=clockevent->tick_rate/HZ;++clockevent->timer.mode=DAVINCI_TIMER_MODE_PERIODIC;+davinci_timer_set_period(&clockevent->timer,period);++return0;+}++staticintdavinci_timer_set_state_oneshot(structclock_event_device*dev)+{+structdavinci_timer_clockevent*clockevent;++clockevent=to_davinci_timer_clockevent(dev);+clockevent->timer.mode=DAVINCI_TIMER_MODE_ONESHOT;++return0;+}++staticvoiddavinci_timer_init(void__iomem*base)+{+/* Set clock to internal mode and disable it. */+__raw_writel(0x0,base+DAVINCI_TIMER_REG_TCR);+/*+*Resetboth32-bittimers,setnoprescalerfortimer34,setthe+*timertodual32-bitunchainedmode,unresetboth32-bittimers.+*/+__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);+}++int__initdavinci_timer_register(structclk*clk,+conststructdavinci_timer_cfg*timer_cfg)+{+structdavinci_timer_clocksource*clocksource;+structdavinci_timer_clockevent*clockevent;+void__iomem*base;+intrv;++rv=clk_prepare_enable(clk);+if(rv){+pr_err("%s: Unable to prepare and enable the timer clock\n",+__func__);+returnrv;+}++base=request_mem_region(timer_cfg->reg.start,+resource_size(&timer_cfg->reg),+"davinci-timer");+if(!base){+pr_err("%s: Unable to request memory region\n",__func__);+return-EBUSY;+}++base=ioremap(timer_cfg->reg.start,resource_size(&timer_cfg->reg));+if(!base){+pr_err("%s: Unable to map the register range\n",__func__);+return-ENOMEM;+}++davinci_timer_init(base);++clockevent=kzalloc(sizeof(*clockevent),GFP_KERNEL);+if(!clockevent){+pr_err("%s: Error allocating memory for clockevent data\n",+__func__);+return-ENOMEM;+}++clockevent->dev.name="tim12";+clockevent->dev.features=CLOCK_EVT_FEAT_ONESHOT;+clockevent->dev.set_next_event=davinci_timer_set_next_event;+clockevent->dev.set_state_shutdown=davinci_timer_set_state_shutdown;+clockevent->dev.set_state_periodic=davinci_timer_set_state_periodic;+clockevent->dev.set_state_oneshot=davinci_timer_set_state_oneshot;+clockevent->dev.cpumask=cpumask_of(0);+clockevent->tick_rate=clk_get_rate(clk);+clockevent->timer.mode=DAVINCI_TIMER_MODE_DISABLED;+clockevent->timer.base=base;+clockevent->timer.regs=&davinci_timer_tim12_regs;++if(timer_cfg->cmp_off){+clockevent->timer.cmp_off=timer_cfg->cmp_off;+clockevent->timer.set_period=davinci_timer_set_period_cmp;+}else{+clockevent->dev.features|=CLOCK_EVT_FEAT_PERIODIC;+clockevent->timer.set_period=davinci_timer_set_period_std;+}++rv=request_irq(timer_cfg->irq[DAVINCI_TIMER_CLOCKEVENT_IRQ].start,+davinci_timer_irq_timer,IRQF_TIMER,+"clockevent",clockevent);+if(rv){+pr_err("%s: Unable to request the clockevent interrupt\n",+__func__);+returnrv;+}++clockevents_config_and_register(&clockevent->dev,+clockevent->tick_rate,+DAVINCI_TIMER_MIN_DELTA,+DAVINCI_TIMER_MAX_DELTA);++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;+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;++rv=request_irq(timer_cfg->irq[DAVINCI_TIMER_CLOCKSOURCE_IRQ].start,+davinci_timer_irq_freerun,IRQF_TIMER,+"free-run counter",clocksource);+if(rv){+pr_err("%s: Unable to request the clocksource interrupt\n",+__func__);+returnrv;+}++rv=clocksource_register_hz(&clocksource->dev,clockevent->tick_rate);+if(rv){+pr_err("%s: Unable to register clocksource\n",+__func__);+returnrv;+}++davinci_timer_clksrc_timer=&clocksource->timer;++sched_clock_register(davinci_timer_read_sched_clock,+DAVINCI_TIMER_CLKSRC_BITS,+clockevent->tick_rate);++davinci_timer_set_period(&clockevent->timer,+clockevent->tick_rate/HZ);+davinci_timer_set_period(&clocksource->timer,UINT_MAX);++return0;+}++staticint__initof_davinci_timer_register(structdevice_node*np)+{+structdavinci_timer_cfgtimer_cfg={};+structclk*clk;+intrv;++rv=of_address_to_resource(np,0,&timer_cfg.reg);+if(rv){+pr_err("%s: Unable to get the register range for timer\n",+__func__);+returnrv;+}++rv=of_irq_to_resource_table(np,timer_cfg.irq,+DAVINCI_TIMER_NUM_IRQS);+if(rv!=DAVINCI_TIMER_NUM_IRQS){+pr_err("%s: Unable to get the interrupts for timer\n",+__func__);+returnrv;+}++clk=of_clk_get(np,0);+if(IS_ERR(clk)){+pr_err("%s: Unable to get the timer clock\n",__func__);+returnPTR_ERR(clk);+}++rv=davinci_timer_register(clk,&timer_cfg);+if(rv)+clk_put(clk);++returnrv;+}+TIMER_OF_DECLARE(davinci_timer,"ti,da830-timer",of_davinci_timer_register);
From: David Lechner <david@lechnology.com> Date: 2019-02-05 02:14:21
On 2/4/19 11:17 AM, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski <redacted>
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 <redacted>
---
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.
It is not obvious to me what the abbreviations tim and prd
mean. Add comments or change the names?
+ unsigned int enamode_shift;
+};
+
...
+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?
+
+ new = orig & ~mask;
+ new |= val & mask;
+
+ davinci_timer_write(timer, reg, new);
+}
...
+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");
+ DAVINCI_TIMER_CLOCKSOURCE_IRQ,
+ DAVINCI_TIMER_NUM_IRQS,
+};
+
+/**
+ * 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.
+ *
+ * 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__ */
From: David Lechner <david@lechnology.com> Date: 2019-02-05 02:37:54
On 2/4/19 11:17 AM, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski <redacted>
Currently the clocksource and clockevent support for davinci platforms
lives in mach-davinci. It hard-codes many things, used global variables,
s/used/uses/
implements functionalities unused by any platform and has code fragments
scattered across many (often unrelated) files.
From: Sekhar Nori <hidden> Date: 2019-02-08 13:24:37
On 04/02/19 10:47 PM, Bartosz Golaszewski wrote:
+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().
+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);
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.
+/**
+ * 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.
+ *
+ * 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.
From: Bartosz Golaszewski <redacted>
We now have a proper clocksource driver for davinci. Switch the platform
to using it.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/da830.c | 49 ++++++++++++++++-------------------
1 file changed, 22 insertions(+), 27 deletions(-)
From: David Lechner <david@lechnology.com> Date: 2019-02-05 02:36:31
On 2/4/19 11:17 AM, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski <redacted>
We now have a proper clocksource driver for davinci. Switch the platform
to using it.
Signed-off-by: Bartosz Golaszewski <redacted>
---
From: Sekhar Nori <hidden> Date: 2019-02-08 12:23:42
On 04/02/19 10:47 PM, Bartosz Golaszewski wrote:
-/*
- * 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
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
pt., 8 lut 2019 o 13:23 Sekhar Nori [off-list ref] napisał(a):
On 04/02/19 10:47 PM, Bartosz Golaszewski wrote:
quoted
-/*
- * 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
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
No it's still the same. I added some comments to the driver's header in v2.
Bart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Bartosz Golaszewski <redacted>
We now have a proper clocksource driver for davinci. Switch the platform
to using it.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/dm365.c | 30 +++++++++++++++++++++++-------
1 file changed, 23 insertions(+), 7 deletions(-)
@@ -1,33 +0,0 @@-/*- * Local header file for DaVinci time code.- *- * Author: Kevin Hilman, MontaVista Software, Inc. <source@mvista.com>- *- * 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 */
From: Bartosz Golaszewski <redacted>
We now have a proper clocksource driver for davinci. Switch the platform
to using it.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/dm646x.c | 36 ++++++++++++++++++++++------------
1 file changed, 23 insertions(+), 13 deletions(-)
@@ -654,6 +662,7 @@ void __init dm646x_init_time(unsigned long ref_clk_rate,{void__iomem*pll1,*psc;structclk*clk;+intrv;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);}staticstructresourcedm646x_pll2_resources[]={
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Bartosz Golaszewski <redacted>
We now have a proper clocksource driver for davinci. Switch the platform
to using it.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/dm644x.c | 36 ++++++++++++++++++++++------------
1 file changed, 23 insertions(+), 13 deletions(-)
From: Bartosz Golaszewski <redacted>
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.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/davinci.h | 3 +++
arch/arm/mach-davinci/include/mach/time.h | 2 --
2 files changed, 3 insertions(+), 2 deletions(-)
From: Bartosz Golaszewski <redacted>
We now have a proper clocksource driver for davinci. Switch the platform
to using it.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/dm355.c | 36 ++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)
From: Sekhar Nori <hidden> Date: 2019-02-08 12:35:06
On 04/02/19 10:47 PM, Bartosz Golaszewski wrote:
-/*
- * 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).
pt., 8 lut 2019 o 13:34 Sekhar Nori [off-list ref] napisał(a):
On 04/02/19 10:47 PM, Bartosz Golaszewski wrote:
quoted
-/*
- * 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).
From: Bartosz Golaszewski <redacted>
We now have a proper clocksource driver for davinci. Switch the platform
to using it.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/da850.c | 56 ++++++++++++++---------------------
1 file changed, 22 insertions(+), 34 deletions(-)
From: David Lechner <david@lechnology.com> Date: 2019-02-05 02:35:07
On 2/4/19 11:17 AM, Bartosz Golaszewski wrote:
quoted hunk
From: Bartosz Golaszewski <redacted>
We now have a proper clocksource driver for davinci. Switch the platform
to using it.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/da850.c | 56 ++++++++++++++---------------------
1 file changed, 22 insertions(+), 34 deletions(-)
SZ_4K - 1
This should have prevented watchdog timer from getting registered.
Thanks,
Sekhar
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
SZ_4K - 1
This 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
SZ_4K - 1
This 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.
Ah, got it. Perhaps a call to request_region() should be added to catch
issues?
Thanks,
Sekhar
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
SZ_4K - 1
This 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.
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
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Bartosz Golaszewski <redacted>
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 <redacted>
---
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(+)
@@ -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);}
@@ -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);}
@@ -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);}
@@ -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);}
@@ -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);}
@@ -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);}
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: David Lechner <david@lechnology.com> Date: 2019-02-05 02:18:45
On 2/4/19 11:17 AM, Bartosz Golaszewski wrote:
quoted hunk
From: Bartosz Golaszewski <redacted>
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 <redacted>
---
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(+)
@@ -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");
wt., 5 lut 2019 o 03:18 David Lechner [off-list ref] napisał(a):
On 2/4/19 11:17 AM, Bartosz Golaszewski wrote:
quoted
From: Bartosz Golaszewski <redacted>
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 <redacted>
---
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(+)
@@ -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?
That will give us the reason AND point us to the right place in the code.
Bart