| Message ID | 20170123135423.28780-3-chris.brandt@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On Mon, Jan 23, 2017 at 08:54:23AM -0500, Chris Brandt wrote: > This patch adds a OSTM driver for the Renesas architecture. As it is a new driver, please give technical details for the log. Replace ioread/write by readl/writel in the code. > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> > > --- > v2: > * changed implementation to be independent channel nodes > --- > arch/arm/mach-shmobile/Kconfig | 1 + > drivers/clocksource/Kconfig | 12 ++ > drivers/clocksource/Makefile | 1 + > drivers/clocksource/renesas-ostm.c | 349 +++++++++++++++++++++++++++++++++++++ > 4 files changed, 363 insertions(+) > create mode 100644 drivers/clocksource/renesas-ostm.c > > diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig > index 2bb4b09..b928634 100644 > --- a/arch/arm/mach-shmobile/Kconfig > +++ b/arch/arm/mach-shmobile/Kconfig > @@ -57,6 +57,7 @@ config ARCH_R7S72100 > select PM > select PM_GENERIC_DOMAINS > select SYS_SUPPORTS_SH_MTU2 > + select SYS_SUPPORTS_RENESAS_OSTM - select SYS_SUPPORTS_RENESAS_OSTM + select RENESAS_OSTM > > config ARCH_R8A73A4 > bool "R-Mobile APE6 (R8A73A40)" > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > index 4866f7a..95c8d56 100644 > --- a/drivers/clocksource/Kconfig > +++ b/drivers/clocksource/Kconfig > @@ -431,6 +431,9 @@ config MTK_TIMER > config SYS_SUPPORTS_SH_MTU2 > bool > > +config SYS_SUPPORTS_RENESAS_OSTM > + bool > + -config SYS_SUPPORTS_RENESAS_OSTM - bool - > config SYS_SUPPORTS_SH_TMU > bool > > @@ -467,6 +470,15 @@ config SH_TIMER_MTU2 > Timer Pulse Unit 2 (MTU2) hardware available on SoCs from Renesas. > This hardware comes with 16 bit-timer registers. > > +config RENESAS_OSTM > + bool "Renesas OSTM timer driver" if COMPILE_TEST > + depends on GENERIC_CLOCKEVENTS > + select CLKSRC_MMIO > + default SYS_SUPPORTS_RENESAS_OSTM - default SYS_SUPPORTS_RENESAS_OSTM Except I missing something, I don' the point of having this intermediate config option. > + help > + This enables the build of the OSTM timer driver. > + It creates a clock source and clock event device. > + > config SH_TIMER_TMU > bool "Renesas TMU timer driver" if COMPILE_TEST > depends on GENERIC_CLOCKEVENTS > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile > index a14111e..bbd163b 100644 > --- a/drivers/clocksource/Makefile > +++ b/drivers/clocksource/Makefile > @@ -8,6 +8,7 @@ obj-$(CONFIG_CS5535_CLOCK_EVENT_SRC) += cs5535-clockevt.o > obj-$(CONFIG_CLKSRC_JCORE_PIT) += jcore-pit.o > obj-$(CONFIG_SH_TIMER_CMT) += sh_cmt.o > obj-$(CONFIG_SH_TIMER_MTU2) += sh_mtu2.o > +obj-$(CONFIG_RENESAS_OSTM) += renesas-ostm.o > obj-$(CONFIG_SH_TIMER_TMU) += sh_tmu.o > obj-$(CONFIG_EM_TIMER_STI) += em_sti.o > obj-$(CONFIG_CLKBLD_I8253) += i8253.o > diff --git a/drivers/clocksource/renesas-ostm.c b/drivers/clocksource/renesas-ostm.c > new file mode 100644 > index 0000000..37f2461 > --- /dev/null > +++ b/drivers/clocksource/renesas-ostm.c > @@ -0,0 +1,349 @@ > +/* > + * Renesas Timer Support - OSTM > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include <linux/platform_device.h> > +#include <linux/clk.h> > +#include <linux/clockchips.h> > +#include <linux/interrupt.h> > +#include <linux/slab.h> > +#include <linux/module.h> Please remove everything module related here and below. This driver is not supposed to be removed and it is compiled in with the Kconfig option. > +#include <linux/pm_runtime.h> > +#include <linux/sched_clock.h> > + > +/* > + * The OSTM contains independent channels. > + * The first OSTM channel probed will be set up as a free running > + * clocksource. Additionally we will use this clocksource for the system > + * schedule timer sched_clock(). > + * > + * The second (or more) channel probed will be set up as an interrupt > + * driven clock event. > + */ > + > +struct ostm_device { > + struct platform_device *pdev; > + int irq; > + struct clk *clk; > + unsigned long rate; > + void __iomem *base; > + unsigned long ticks_per_jiffy; > + struct clock_event_device ced; > +}; You can probably reduce the size of this structure by removing some fields which are used only at init time. > + > +static void __iomem *system_clock; /* For sched_clock() */ > + > +/* OSTM REGISTERS */ > +#define OSTM_CMP 0x000 /* RW,32 */ > +#define OSTM_CNT 0x004 /* R,32 */ > +#define OSTM_TE 0x010 /* R,8 */ > +#define OSTM_TS 0x014 /* W,8 */ > +#define OSTM_TT 0x018 /* W,8 */ > +#define OSTM_CTL 0x020 /* RW,8 */ > + > +#define TE 0x01 > +#define TS 0x01 > +#define TT 0x01 > +#define CTL_PERIODIC 0x00 > +#define CTL_ONESHOT 0x02 > +#define CTL_FREERUN 0x02 > + > +static struct ostm_device *ced_to_ostm(struct clock_event_device *ced) > +{ > + return container_of(ced, struct ostm_device, ced); > +} > + > +static int __init ostm_init_clksrc(struct ostm_device *ostm) > +{ > + int ret; > + > + /* irq not used (clock sources don't use interrupts) */ > + > + /* stop counter */ One line comment is usually in the network code. Please use multiline. /* * Bla bla */ > + iowrite8(TT, ostm->base + OSTM_TT); > + while (ioread8(ostm->base + OSTM_TE) & TE) > + ; Comment here for this dangerous loop. Explain why we can't be stuck here. > + > + /* setup as freerun */ > + iowrite32(0, ostm->base + OSTM_CMP); > + iowrite8(CTL_FREERUN, ostm->base + OSTM_CTL); > + iowrite8(TS, ostm->base + OSTM_TS); > + > + /* register */ > + ret = clocksource_mmio_init(ostm->base + OSTM_CNT, > + "ostm", ostm->rate, > + 300, 32, clocksource_mmio_readl_up); > + > + return ret; return clocksource_mmio_init(...); > +} > + > +static u64 notrace ostm_read_sched_clock(void) > +{ > + return ioread32(system_clock); > +} > + > +static int __init ostm_init_sched_clock(struct ostm_device *ostm) > +{ > + unsigned long flags; > + > + system_clock = ostm->base + OSTM_CNT; > + local_irq_save(flags); > + local_irq_disable(); 1. local_irq_disable() is not needed, local_irq_save() saves the irq flags and disables the irq. 2. Why do you need to disable the irq here ? > + sched_clock_register(ostm_read_sched_clock, 32, ostm->rate); > + local_irq_restore(flags); > + > + return 0; > +} > + > +static void ostm_clkevt_timer_stop(struct ostm_device *ostm) > +{ > + if (ioread8(ostm->base + OSTM_TE) & TE) { > + iowrite8(TT, ostm->base + OSTM_TT); > + while (ioread8(ostm->base + OSTM_TE) & TE) > + ; Same comment as above. > + } > +} > + > +static int ostm_clock_event_next(unsigned long delta, > + struct clock_event_device *ced) > +{ > + struct ostm_device *ostm = ced_to_ostm(ced); > + > + WARN_ON(!clockevent_state_oneshot(ced)); Pointless check. It is up to the time framework to handle that and that is the case. > + > + ostm_clkevt_timer_stop(ostm); > + > + iowrite32(delta, ostm->base + OSTM_CMP); > + iowrite8(CTL_ONESHOT, ostm->base + OSTM_CTL); > + iowrite8(TS, ostm->base + OSTM_TS); > + > + return 0; > +} > + > +static int ostm_shutdown(struct clock_event_device *ced) > +{ > + struct ostm_device *ostm = ced_to_ostm(ced); > + > + ostm_clkevt_timer_stop(ostm); > + > + return 0; > +} > +static int ostm_set_periodic(struct clock_event_device *ced) > +{ > + struct ostm_device *ostm = ced_to_ostm(ced); > + > + if (clockevent_state_oneshot(ced) || clockevent_state_periodic(ced)) > + ostm_clkevt_timer_stop(ostm); > + > + iowrite32(ostm->ticks_per_jiffy - 1, ostm->base + OSTM_CMP); > + iowrite8(CTL_PERIODIC, ostm->base + OSTM_CTL); > + iowrite8(TS, ostm->base + OSTM_TS); > + > + return 0; > +} > + > +static int ostm_set_oneshot(struct clock_event_device *ced) > +{ > + struct ostm_device *ostm = ced_to_ostm(ced); > + > + ostm_clkevt_timer_stop(ostm); > + > + return 0; > +} > + > +static irqreturn_t ostm_timer_interrupt(int irq, void *dev_id) > +{ > + struct ostm_device *ostm = dev_id; > + > + if (clockevent_state_oneshot(&ostm->ced)) > + ostm_clkevt_timer_stop(ostm); > + > + /* notify clockevent layer */ > + if (ostm->ced.event_handler) > + ostm->ced.event_handler(&ostm->ced); > + > + return IRQ_HANDLED; > +} > + > +static int __init ostm_init_clkevt(struct ostm_device *ostm) > +{ > + struct clock_event_device *ced = &ostm->ced; > + int ret = -ENXIO; > + > + ret = request_irq(ostm->irq, ostm_timer_interrupt, > + IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING, > + dev_name(&ostm->pdev->dev), ostm); devm_request_irq Are you sure the IRQF_NOBALANCING flag should be used ? By default the interrupt is pinned to cpu0 below. If this timer is used as a broadcast timer for a system with deep idle, you may want to add the DYNIRQ flag feature to improve the wakeups on the system. > + if (ret) { > + dev_err(&ostm->pdev->dev, "failed to request irq\n"); > + return ret; > + } > + > + ced->name = "ostm"; > + ced->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC; > + ced->set_state_shutdown = ostm_shutdown; > + ced->set_state_periodic = ostm_set_periodic; > + ced->set_state_oneshot = ostm_set_oneshot; > + ced->set_next_event = ostm_clock_event_next; > + ced->shift = 32; > + ced->rating = 300; > + ced->cpumask = cpumask_of(0); > + clockevents_config_and_register(ced, ostm->rate, 0xf, 0xffffffff); > + > + return 0; > +} > + > +static int __init ostm_probe(struct platform_device *pdev) > +{ > + struct ostm_device *ostm; > + struct resource *res; > + int ret = -EFAULT; > + > + if (!is_early_platform_device(pdev)) { > + pm_runtime_set_active(&pdev->dev); > + pm_runtime_enable(&pdev->dev); > + } Can you clarify why the 'early' is needed here ? I don't see the pm_runtime_get/pm_runtime_put in the corresponding function. What is the benefit of using pm_runtime in this driver ? Isn't the timer supposed to be in an always-on power domain ? > + ostm = platform_get_drvdata(pdev); > + if (ostm) { > + dev_info(&pdev->dev, "kept as earlytimer\n"); > + ret = 0; > + goto out; > + } > + > + ostm = kzalloc(sizeof(*ostm), GFP_KERNEL); devm_kzalloc ? > + if (!ostm) { > + dev_err(&pdev->dev, "failed to allocate memory\n"); A memory failure allocation always triggers a dumpstack (IIRC), so this error message is pointless. > + return -ENOMEM; > + } > + > + ostm->pdev = pdev; > + platform_set_drvdata(ostm->pdev, ostm); > + > + res = platform_get_resource(ostm->pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(&ostm->pdev->dev, "failed to get I/O memory\n"); > + goto err; > + } > + > + ostm->base = ioremap_nocache(res->start, resource_size(res)); devm_ioremap_nocache ? > + if (!ostm->base) { > + dev_err(&ostm->pdev->dev, "failed to remap I/O memory\n"); > + goto err; > + } > + > + ostm->irq = platform_get_irq(ostm->pdev, 0); > + if (ostm->irq < 0) { > + dev_err(&ostm->pdev->dev, "failed to get irq\n"); > + goto err; > + } > + > + ostm->clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(ostm->clk)) { > + dev_err(&ostm->pdev->dev, "failed to get clock\n"); > + ostm->clk = NULL; > + goto err; > + } > + > + ret = clk_prepare_enable(ostm->clk); > + if (ret) { > + dev_err(&ostm->pdev->dev, "failed to enable clock\n"); > + goto err; > + } > + > + ostm->rate = clk_get_rate(ostm->clk); > + ostm->ticks_per_jiffy = (ostm->rate + HZ / 2) / HZ; > + > + /* First probed device will be used as system clocksource */ > + if (!system_clock) { > + /* use as clocksource */ > + ret = ostm_init_clksrc(ostm); > + > + /* use as system scheduling clock */ > + if (!ret) > + ret = ostm_init_sched_clock(ostm); > + > + if (ret) { > + dev_err(&pdev->dev, "failed to use as sched_clock\n"); > + system_clock = (void *)-1; /* prevent future attempts */ > + ret = 0; /* still works as clocksource */ > + } This error code check is unnecessary complex. ostm_init_sched_clock always return zero. if (!system_clock) { system_clock = ostm_init_sched_clock(ostm); ret = ostm_init_clksrc(ostm) if (ret) dev_err("Failed to initialize the clocksource\n"); } else { ... } This is not perfect but until I send the clksrc_of/clkevt_of split changes, we have to deal with these kind of hacks. > + if (!ret) > + dev_info(&pdev->dev, "used for clocksource\n"); > + } else { > + /* use as clock event */ > + ret = ostm_init_clkevt(ostm); > + > + if (!ret) > + dev_info(&pdev->dev, "used for clock events\n"); > + } > + > +err: > + if (ret) { > + if (ostm->clk) > + clk_disable_unprepare(ostm->clk); > + if (ostm->base) > + iounmap(ostm->base); > + kfree(ostm); > + platform_set_drvdata(pdev, NULL); As iomap, kzalloc were done with the devm, then the rollback will be handled with the device framework. > + pm_runtime_idle(&pdev->dev); > + return ret; > + } > + > + if (is_early_platform_device(pdev)) > + return ret; > + > +out: > + pm_runtime_irq_safe(&pdev->dev); > + > + return ret; > +} > + > +static int ostm_remove(struct platform_device *pdev) > +{ > + return -EBUSY; /* cannot unregister clockevent */ > +} > + > +static const struct of_device_id ostm_of_table[] __maybe_unused = { > + { .compatible = "renesas,ostm" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ostm_of_table); > + > +static struct platform_driver ostm_timer = { > + .probe = ostm_probe, > + .remove = ostm_remove, > + .driver = { > + .name = "ostm", > + .of_match_table = of_match_ptr(ostm_of_table), > + }, > +}; > + > +static int __init ostm_init(void) > +{ > + return platform_driver_register(&ostm_timer); > +} > + > +static void __exit ostm_exit(void) > +{ > + platform_driver_unregister(&ostm_timer); > +} > + > +early_platform_init("earlytimer", &ostm_timer); > +subsys_initcall(ostm_init); > +module_exit(ostm_exit); > + > +MODULE_AUTHOR("Chris Brandt"); > +MODULE_DESCRIPTION("Renesas OSTM Timer Driver"); > +MODULE_LICENSE("GPL v2"); Maybe you can try with builtin_platform ? -- Daniel
Hello Daniel, Thank you for the review. Your suggestions were very helpful to me. I basically made all the changes, but I had some questions on the Kconfig and multi-line commenting. On Monday, January 23, 2017, Daniel Lezcano wrote: > > This patch adds a OSTM driver for the Renesas architecture. > > As it is a new driver, please give technical details for the log. OK. > Replace ioread/write by readl/writel in the code. OK. > > @@ -467,6 +470,15 @@ config SH_TIMER_MTU2 > > Timer Pulse Unit 2 (MTU2) hardware available on SoCs from Renesas. > > This hardware comes with 16 bit-timer registers. > > > > +config RENESAS_OSTM > > + bool "Renesas OSTM timer driver" if COMPILE_TEST > > + depends on GENERIC_CLOCKEVENTS > > + select CLKSRC_MMIO > > + default SYS_SUPPORTS_RENESAS_OSTM > > - default SYS_SUPPORTS_RENESAS_OSTM > > Except I missing something, I don' the point of having this intermediate > config option. I was basically following what all the other clocksource drivers do. Selecting an ARCH will then set SYS_SUPPORTS_xxTIMERxx which will then force xxTIMERxx=y. But, if "COMPILE_TEST" is set, you can choose what clocksource timers you want to build in. Is your suggestion to do away with the COMPILE_TEST ability and just force RENESAS_OSTM=y if ARCH_R7S72100 is selected? > > +#include <linux/module.h> > > Please remove everything module related here and below. This driver is not > supposed to be removed and it is compiled in with the Kconfig option. Good point. I will remove. I guess if you can't compile it as a module, you don't need things like 'MODULE_LICENSE'. > > +struct ostm_device { > > + struct platform_device *pdev; > > + int irq; > > + struct clk *clk; > > + unsigned long rate; > > + void __iomem *base; > > + unsigned long ticks_per_jiffy; > > + struct clock_event_device ced; > > +}; > > You can probably reduce the size of this structure by removing some fields > which are used only at init time. Looks like I can get rid of 'clk', 'irq' and 'rate'. Thanks. > > +static int __init ostm_init_clksrc(struct ostm_device *ostm) { > > + int ret; > > + > > + /* irq not used (clock sources don't use interrupts) */ > > + > > + /* stop counter */ > > One line comment is usually in the network code. Please use multiline. > > /* > * Bla bla > */ I'm confused. Do you mean: A) use multiline formatting for every single-line comment throughout the file? B) use multiline for any place where you have 2 or more single-line comments back to back? > > + iowrite8(TT, ostm->base + OSTM_TT); > > + while (ioread8(ostm->base + OSTM_TE) & TE) > > + ; > > Comment here for this dangerous loop. Explain why we can't be stuck here. OK, I'll add something in as it's pretty safe. > > > + > > + /* setup as freerun */ > > + iowrite32(0, ostm->base + OSTM_CMP); > > + iowrite8(CTL_FREERUN, ostm->base + OSTM_CTL); > > + iowrite8(TS, ostm->base + OSTM_TS); > > + > > + /* register */ > > + ret = clocksource_mmio_init(ostm->base + OSTM_CNT, > > + "ostm", ostm->rate, > > + 300, 32, clocksource_mmio_readl_up); > > + > > + return ret; > > return clocksource_mmio_init(...); OK. > > +static int __init ostm_init_sched_clock(struct ostm_device *ostm) { > > + unsigned long flags; > > + > > + system_clock = ostm->base + OSTM_CNT; > > + local_irq_save(flags); > > + local_irq_disable(); > > 1. local_irq_disable() is not needed, local_irq_save() saves the irq flags > and disables the irq. > > 2. Why do you need to disable the irq here ? OK, I see that none of the other drivers are disabling irq, so I'll take that code out. Thanks. > > +static int ostm_clock_event_next(unsigned long delta, > > + struct clock_event_device *ced) { > > + struct ostm_device *ostm = ced_to_ostm(ced); > > + > > + WARN_ON(!clockevent_state_oneshot(ced)); > > Pointless check. It is up to the time framework to handle that and that is > the case. OK. I saw other drivers doing it so I thought it was my responsibility. I'll take that lineout. > > +static int __init ostm_init_clkevt(struct ostm_device *ostm) { > > + struct clock_event_device *ced = &ostm->ced; > > + int ret = -ENXIO; > > + > > + ret = request_irq(ostm->irq, ostm_timer_interrupt, > > + IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING, > > + dev_name(&ostm->pdev->dev), ostm); > > devm_request_irq > > Are you sure the IRQF_NOBALANCING flag should be used ? By default the > interrupt is pinned to cpu0 below. If this timer is used as a broadcast > timer for a system with deep idle, you may want to add the DYNIRQ flag > feature to improve the wakeups on the system. OK, thank you. After some reading, I understand IRQF_NOBALANCING better. You're point is valid: Regardless of the fact that the SoCs I'll be using this on are all single core, like you said below the cpu mask is set to cpu0. I will remove IRQF_NOBALANCING. > > +static int __init ostm_probe(struct platform_device *pdev) { > > + struct ostm_device *ostm; > > + struct resource *res; > > + int ret = -EFAULT; > > + > > + if (!is_early_platform_device(pdev)) { > > + pm_runtime_set_active(&pdev->dev); > > + pm_runtime_enable(&pdev->dev); > > + } > > Can you clarify why the 'early' is needed here ? Actually, it means nothing. I was using sh_mtu2.c and sh_cmt.c as reference and they were registering an "earlytimer" which made me think the kernel would probe this driver early in boot....but....now I see that is just a SH4 kernel specific thing. So, I will remove all the early platform reference stuff. Of course I still have the question of how are you supposed to get a clocksource up and running early in boot. (but I'll figure that out later). > I don't see the pm_runtime_get/pm_runtime_put in the corresponding > function. > > What is the benefit of using pm_runtime in this driver ? Isn't the timer > supposed to be in an always-on power domain ? When you register a clocksource, you can specify a .enable and .disable callback where you can do runtime_pm. However... A. I'm using clocksource_mmio_init, so no enable/dispable option is possible. B. I just found out today that runtime pm is not going to work well on the SoC this driver was intended for, so it pointless anyway. I'll remove all the runtime pm stuff. Thank you for point this out. > > + ostm = platform_get_drvdata(pdev); > > + if (ostm) { > > + dev_info(&pdev->dev, "kept as earlytimer\n"); > > + ret = 0; > > + goto out; > > + } > > + > > + ostm = kzalloc(sizeof(*ostm), GFP_KERNEL); > > devm_kzalloc ? OK, thank you. Means I can also get rid of '#include <linux/slab.h>' > > + if (!ostm) { > > + dev_err(&pdev->dev, "failed to allocate memory\n"); > > A memory failure allocation always triggers a dumpstack (IIRC), so this > error message is pointless. OK, removed. > > + return -ENOMEM; > > + } > > + > > + ostm->pdev = pdev; > > + platform_set_drvdata(ostm->pdev, ostm); > > + > > + res = platform_get_resource(ostm->pdev, IORESOURCE_MEM, 0); > > + if (!res) { > > + dev_err(&ostm->pdev->dev, "failed to get I/O memory\n"); > > + goto err; > > + } > > + > > + ostm->base = ioremap_nocache(res->start, resource_size(res)); > > devm_ioremap_nocache ? OK, thank you. Just need to also add '#include <linux/io.h>' (looks like devm_ioremap and devm_ioremap_nocache is not as popular as devm_kzalloc for some reason) > > + /* First probed device will be used as system clocksource */ > > + if (!system_clock) { > > + /* use as clocksource */ > > + ret = ostm_init_clksrc(ostm); > > + > > + /* use as system scheduling clock */ > > + if (!ret) > > + ret = ostm_init_sched_clock(ostm); > > + > > + if (ret) { > > + dev_err(&pdev->dev, "failed to use as sched_clock\n"); > > + system_clock = (void *)-1; /* prevent future attempts */ > > + ret = 0; /* still works as clocksource */ > > + } > > This error code check is unnecessary complex. ostm_init_sched_clock always > return zero. Good point. And since sched_clock_register() always returns void, ostm_init_sched_clock might was well return void as well. > > +err: > > + if (ret) { > > + if (ostm->clk) > > + clk_disable_unprepare(ostm->clk); > > + if (ostm->base) > > + iounmap(ostm->base); > > + kfree(ostm); > > + platform_set_drvdata(pdev, NULL); > > As iomap, kzalloc were done with the devm, then the rollback will be > handled with the device framework. I didn't realize that. Thank you. > > +static int __init ostm_init(void) > > +{ > > + return platform_driver_register(&ostm_timer); > > +} > > + > > +static void __exit ostm_exit(void) > > +{ > > + platform_driver_unregister(&ostm_timer); > > +} > > + > > +early_platform_init("earlytimer", &ostm_timer); > > +subsys_initcall(ostm_init); module_exit(ostm_exit); > > + > > +MODULE_AUTHOR("Chris Brandt"); > > +MODULE_DESCRIPTION("Renesas OSTM Timer Driver"); MODULE_LICENSE("GPL > > +v2"); > > Maybe you can try with builtin_platform ? Good idea. But, now I get a "Section mismatch" during link time so I'll have to figure out why that is. Thank you, Chris
On Tue, Jan 24, 2017 at 04:45:47AM +0000, Chris Brandt wrote: Hi Chris, [ ... ] > > > + bool "Renesas OSTM timer driver" if COMPILE_TEST > > > + depends on GENERIC_CLOCKEVENTS > > > + select CLKSRC_MMIO > > > + default SYS_SUPPORTS_RENESAS_OSTM > > > > - default SYS_SUPPORTS_RENESAS_OSTM > > > > Except I missing something, I don' the point of having this intermediate > > config option. > > I was basically following what all the other clocksource drivers do. > Selecting an ARCH will then set SYS_SUPPORTS_xxTIMERxx which will then > force xxTIMERxx=y. Yeah, little by little these options are clean up to be more consistent across the different drivers. The sh/mtu drivers are different from the other drivers. The idea with the config option is to let the platform to select the drivers, and optionnaly allows the compilation on other architecture to increase the compilation test coverage. That is the reason of COMPILE_TEST. > But, if "COMPILE_TEST" is set, you can choose what clocksource timers > you want to build in. > > Is your suggestion to do away with the COMPILE_TEST ability and > just force RENESAS_OSTM=y if ARCH_R7S72100 is selected? Just like that: config RENESAS_OSTM bool "Renesas OSTM timer driver" if COMPILE_TEST depends on GENERIC_CLOCKEVENTS select CLKSRC_MMIO help Enables the support for the Renesas OSTM And then from arch/arm/mach-shmobile/Kconfig: select RENESAS_OSTM > > > +#include <linux/module.h> > > > > Please remove everything module related here and below. This driver is not > > supposed to be removed and it is compiled in with the Kconfig option. > > Good point. I will remove. > > I guess if you can't compile it as a module, you don't need things like > 'MODULE_LICENSE'. Right. [ ... ] > > > +static int __init ostm_init_clksrc(struct ostm_device *ostm) { > > > + int ret; > > > + > > > + /* irq not used (clock sources don't use interrupts) */ > > > + > > > + /* stop counter */ > > > > One line comment is usually in the network code. Please use multiline. > > > > /* > > * Bla bla > > */ > > I'm confused. Do you mean: > > A) use multiline formatting for every single-line comment throughout > the file? > > B) use multiline for any place where you have 2 or more single-line > comments back to back? I think it is simpler if you just ignore my comment, remove all your comments in this function, implement a couple of wrapper (eg. clksrc_stop, clksrc_start) which will speak by themselves. By the way, is it normal stopping the clockevent is the same code than stopping the clocksource ? [ ... ] > > > + if (!is_early_platform_device(pdev)) { > > > + pm_runtime_set_active(&pdev->dev); > > > + pm_runtime_enable(&pdev->dev); > > > + } > > > > Can you clarify why the 'early' is needed here ? > > Actually, it means nothing. > > I was using sh_mtu2.c and sh_cmt.c as reference and they were registering > an "earlytimer" which made me think the kernel would probe this driver > early in boot....but....now I see that is just a SH4 kernel specific thing. > > So, I will remove all the early platform reference stuff. > > Of course I still have the question of how are you supposed to get a > clocksource up and running early in boot. (but I'll figure that out later). Until the hardware clocksource is registered, the jiffies are used as source of time. That happens at the very early boot time. The clockevent must be registered early to update the jiffies but you won't have to care about that if you use the macro below. [ ... ] > > > +early_platform_init("earlytimer", &ostm_timer); > > > +subsys_initcall(ostm_init); module_exit(ostm_exit); > > > + > > > +MODULE_AUTHOR("Chris Brandt"); > > > +MODULE_DESCRIPTION("Renesas OSTM Timer Driver"); MODULE_LICENSE("GPL > > > +v2"); > > > > Maybe you can try with builtin_platform ? > > Good idea. But, now I get a "Section mismatch" during link time so I'll > have to figure out why that is. Mmh, I think it would be more consistent to convert this to: CLOCKSOURCE_OF_DECLARE(ostm, "renesas,ostm", ostm_init); The only problem is to get the struct device associated to the of_node passed as parameter to ostm_init in order to use the devm_* API. I think of_find_device_by_node should return the platform_device, then pdev->dev. If that works the other drivers will benefit from that to remove all the rollback code everywhere. -- Daniel
Hi Daniel, On Tuesday, January 24, 2017, Daniel Lezcano wrote: > > > > +early_platform_init("earlytimer", &ostm_timer); > > > > +subsys_initcall(ostm_init); module_exit(ostm_exit); > > > > + > > > > +MODULE_AUTHOR("Chris Brandt"); > > > > +MODULE_DESCRIPTION("Renesas OSTM Timer Driver"); > > > > +MODULE_LICENSE("GPL v2"); > > > > > > Maybe you can try with builtin_platform ? > > > > Good idea. But, now I get a "Section mismatch" during link time so > > I'll have to figure out why that is. > > Mmh, I think it would be more consistent to convert this to: > > CLOCKSOURCE_OF_DECLARE(ostm, "renesas,ostm", ostm_init); > > The only problem is to get the struct device associated to the of_node > passed as parameter to ostm_init in order to use the devm_* API. > > I think of_find_device_by_node should return the platform_device, then > pdev->dev. If that works the other drivers will benefit from that to > pdev->remove all > the rollback code everywhere. So I realized that in order to use builtin_platform, I can't have any of the functions in __init because the build system has no idea that I never plan on removing or probing again after boot. But, even if I take out all the __init from my code, I'm still calling clocksource_mmio_init which is __init so I can never escape the "Section mismatch". This leaves me with 2 options: 1) Pull the clocksource_mmio driver code into my driver and leave everything out of __init 2) Rewrite as a DT driver using CLOCKSOURCE_OF_DECLARE which puts most of the code in __init Of course the better (lower system memory) answer is #2. I'll try your trick about of_find_device_by_node because while there are equivalent OF functions for mapping resources, I do like the auto-rollback feature of demv Thank you, Chris
Hi Daniel, On Tuesday, January 24, 2017, Daniel Lezcano wrote: > > > > +early_platform_init("earlytimer", &ostm_timer); > > > > +subsys_initcall(ostm_init); module_exit(ostm_exit); > > > > + > > > > +MODULE_AUTHOR("Chris Brandt"); > > > > +MODULE_DESCRIPTION("Renesas OSTM Timer Driver"); > > > > +MODULE_LICENSE("GPL v2"); > > > > > > Maybe you can try with builtin_platform ? > > > > Good idea. But, now I get a "Section mismatch" during link time so > > I'll have to figure out why that is. > > Mmh, I think it would be more consistent to convert this to: > > CLOCKSOURCE_OF_DECLARE(ostm, "renesas,ostm", ostm_init); > > The only problem is to get the struct device associated to the of_node > passed as parameter to ostm_init in order to use the devm_* API. > > I think of_find_device_by_node should return the platform_device, then > pdev->dev. If that works the other drivers will benefit from that to > pdev->remove all > the rollback code everywhere. It was a good idea....but it will not work. While CLOCKSOURCE_OF_DECLARE is good at putting the driver at the front of the line for loading, it's too early in boot to detect a platform_device. of_find_device_by_node calls bus_find_device. But the first thing that bus_find_device does is: if (!bus || !bus->p) return NULL; But bus->p=0 so it returns immediately. Of course changing the code over to: devm_kzalloc -> devm_kzalloc devm_ioremap_nocache -> of_io_request_and_map platform_get_irq -> irq_of_parse_and_map dev_err -> pr_err Then things work, but I'm back to managing the rollback code manually. Any other ideas on how to get the corresponding platform_device for a DT node? Chris
Hi Chris, On Tue, Jan 24, 2017 at 3:43 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote: > On Tuesday, January 24, 2017, Daniel Lezcano wrote: >> > > > +early_platform_init("earlytimer", &ostm_timer); >> > > > +subsys_initcall(ostm_init); module_exit(ostm_exit); >> > > > + >> > > > +MODULE_AUTHOR("Chris Brandt"); >> > > > +MODULE_DESCRIPTION("Renesas OSTM Timer Driver"); >> > > > +MODULE_LICENSE("GPL v2"); >> > > >> > > Maybe you can try with builtin_platform ? >> > >> > Good idea. But, now I get a "Section mismatch" during link time so >> > I'll have to figure out why that is. >> >> Mmh, I think it would be more consistent to convert this to: >> >> CLOCKSOURCE_OF_DECLARE(ostm, "renesas,ostm", ostm_init); >> >> The only problem is to get the struct device associated to the of_node >> passed as parameter to ostm_init in order to use the devm_* API. >> >> I think of_find_device_by_node should return the platform_device, then >> pdev->dev. If that works the other drivers will benefit from that to >> pdev->remove all >> the rollback code everywhere. > > So I realized that in order to use builtin_platform, I can't have any of the > functions in __init because the build system has no idea that I never plan > on removing or probing again after boot. But, even if I take out all the > __init from my code, I'm still calling clocksource_mmio_init which is __init > so I can never escape the "Section mismatch". For single-probe drivers, you can use builtin_platform_driver_probe(). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, Jan 24, 2017 at 9:19 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote: > On Tuesday, January 24, 2017, Daniel Lezcano wrote: >> > > > +early_platform_init("earlytimer", &ostm_timer); >> > > > +subsys_initcall(ostm_init); module_exit(ostm_exit); >> > > > + >> > > > +MODULE_AUTHOR("Chris Brandt"); >> > > > +MODULE_DESCRIPTION("Renesas OSTM Timer Driver"); >> > > > +MODULE_LICENSE("GPL v2"); >> > > >> > > Maybe you can try with builtin_platform ? >> > >> > Good idea. But, now I get a "Section mismatch" during link time so >> > I'll have to figure out why that is. >> >> Mmh, I think it would be more consistent to convert this to: >> >> CLOCKSOURCE_OF_DECLARE(ostm, "renesas,ostm", ostm_init); >> >> The only problem is to get the struct device associated to the of_node >> passed as parameter to ostm_init in order to use the devm_* API. >> >> I think of_find_device_by_node should return the platform_device, then >> pdev->dev. If that works the other drivers will benefit from that to >> pdev->remove all >> the rollback code everywhere. > > It was a good idea....but it will not work. > > While CLOCKSOURCE_OF_DECLARE is good at putting the driver at the > front of the line for loading, it's too early in boot to detect > a platform_device. That's correct. All those *_OF_DECLARE() style initializations start to break as soon as power and/or clock domains are involved. That's one reason why some subsystems (e.g. clock) are moving away from it. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, Jan 24, 2017 at 08:19:50PM +0000, Chris Brandt wrote: > Hi Daniel, > > On Tuesday, January 24, 2017, Daniel Lezcano wrote: > > > > > +early_platform_init("earlytimer", &ostm_timer); > > > > > +subsys_initcall(ostm_init); module_exit(ostm_exit); > > > > > + > > > > > +MODULE_AUTHOR("Chris Brandt"); > > > > > +MODULE_DESCRIPTION("Renesas OSTM Timer Driver"); > > > > > +MODULE_LICENSE("GPL v2"); > > > > > > > > Maybe you can try with builtin_platform ? > > > > > > Good idea. But, now I get a "Section mismatch" during link time so > > > I'll have to figure out why that is. > > > > Mmh, I think it would be more consistent to convert this to: > > > > CLOCKSOURCE_OF_DECLARE(ostm, "renesas,ostm", ostm_init); > > > > The only problem is to get the struct device associated to the of_node > > passed as parameter to ostm_init in order to use the devm_* API. > > > > I think of_find_device_by_node should return the platform_device, then > > pdev->dev. If that works the other drivers will benefit from that to > > pdev->remove all > > the rollback code everywhere. > > > It was a good idea....but it will not work. > > While CLOCKSOURCE_OF_DECLARE is good at putting the driver at the > front of the line for loading, it's too early in boot to detect > a platform_device. > > of_find_device_by_node calls bus_find_device. But the first thing that > bus_find_device does is: > > if (!bus || !bus->p) > return NULL; > > But bus->p=0 so it returns immediately. > > > Of course changing the code over to: > devm_kzalloc -> devm_kzalloc > devm_ioremap_nocache -> of_io_request_and_map > platform_get_irq -> irq_of_parse_and_map > dev_err -> pr_err > > > Then things work, but I'm back to managing the rollback code manually. > > > Any other ideas on how to get the corresponding platform_device for > a DT node? No :/ So up to you. - CLOCKSOURCE_OF_DECLARE consistent but need rollback or - platform_device but with another timer available at early time
Hi Geert, On Wednesday, January 25, 2017, Geert Uytterhoeven wrote: > > So I realized that in order to use builtin_platform, I can't have any > > of the functions in __init because the build system has no idea that I > > never plan on removing or probing again after boot. But, even if I > > take out all the __init from my code, I'm still calling > > clocksource_mmio_init which is __init so I can never escape the "Section > mismatch". > > For single-probe drivers, you can use builtin_platform_driver_probe(). Thank you for the info. Chris
Hi Daniel, On Wednesday, January 25, 2017, Daniel Lezcano wrote: > > Then things work, but I'm back to managing the rollback code manually. > > > > > > Any other ideas on how to get the corresponding platform_device for a > > DT node? > > No :/ > > So up to you. > - CLOCKSOURCE_OF_DECLARE consistent but need rollback > or > - platform_device but with another timer available at early time As far as I can tell, the rollback functions don't mind if I pass NULL pointers to them. So with CLOCKSOURCE_OF_DECLARE, my error rollback at the end of ostm_init is basically: err: if (ret) { clk_disable_unprepare(ostm_clk); iounmap(ostm->base); kfree(ostm); return ret; } return 0; } If I go with CLOCKSOURCE_OF_DECLARE, I can at least get rid of the early boot message "clocksource_probe: no matching clocksources found" I'll go ahead and send a v4 today with all the changes you suggested. Thank you for your help. Regards, Chris
diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig index 2bb4b09..b928634 100644 --- a/arch/arm/mach-shmobile/Kconfig +++ b/arch/arm/mach-shmobile/Kconfig @@ -57,6 +57,7 @@ config ARCH_R7S72100 select PM select PM_GENERIC_DOMAINS select SYS_SUPPORTS_SH_MTU2 + select SYS_SUPPORTS_RENESAS_OSTM config ARCH_R8A73A4 bool "R-Mobile APE6 (R8A73A40)" diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index 4866f7a..95c8d56 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -431,6 +431,9 @@ config MTK_TIMER config SYS_SUPPORTS_SH_MTU2 bool +config SYS_SUPPORTS_RENESAS_OSTM + bool + config SYS_SUPPORTS_SH_TMU bool @@ -467,6 +470,15 @@ config SH_TIMER_MTU2 Timer Pulse Unit 2 (MTU2) hardware available on SoCs from Renesas. This hardware comes with 16 bit-timer registers. +config RENESAS_OSTM + bool "Renesas OSTM timer driver" if COMPILE_TEST + depends on GENERIC_CLOCKEVENTS + select CLKSRC_MMIO + default SYS_SUPPORTS_RENESAS_OSTM + help + This enables the build of the OSTM timer driver. + It creates a clock source and clock event device. + config SH_TIMER_TMU bool "Renesas TMU timer driver" if COMPILE_TEST depends on GENERIC_CLOCKEVENTS diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile index a14111e..bbd163b 100644 --- a/drivers/clocksource/Makefile +++ b/drivers/clocksource/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_CS5535_CLOCK_EVENT_SRC) += cs5535-clockevt.o obj-$(CONFIG_CLKSRC_JCORE_PIT) += jcore-pit.o obj-$(CONFIG_SH_TIMER_CMT) += sh_cmt.o obj-$(CONFIG_SH_TIMER_MTU2) += sh_mtu2.o +obj-$(CONFIG_RENESAS_OSTM) += renesas-ostm.o obj-$(CONFIG_SH_TIMER_TMU) += sh_tmu.o obj-$(CONFIG_EM_TIMER_STI) += em_sti.o obj-$(CONFIG_CLKBLD_I8253) += i8253.o diff --git a/drivers/clocksource/renesas-ostm.c b/drivers/clocksource/renesas-ostm.c new file mode 100644 index 0000000..37f2461 --- /dev/null +++ b/drivers/clocksource/renesas-ostm.c @@ -0,0 +1,349 @@ +/* + * Renesas Timer Support - OSTM + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <linux/platform_device.h> +#include <linux/clk.h> +#include <linux/clockchips.h> +#include <linux/interrupt.h> +#include <linux/slab.h> +#include <linux/module.h> +#include <linux/pm_runtime.h> +#include <linux/sched_clock.h> + +/* + * The OSTM contains independent channels. + * The first OSTM channel probed will be set up as a free running + * clocksource. Additionally we will use this clocksource for the system + * schedule timer sched_clock(). + * + * The second (or more) channel probed will be set up as an interrupt + * driven clock event. + */ + +struct ostm_device { + struct platform_device *pdev; + + int irq; + struct clk *clk; + unsigned long rate; + void __iomem *base; + unsigned long ticks_per_jiffy; + struct clock_event_device ced; +}; + +static void __iomem *system_clock; /* For sched_clock() */ + +/* OSTM REGISTERS */ +#define OSTM_CMP 0x000 /* RW,32 */ +#define OSTM_CNT 0x004 /* R,32 */ +#define OSTM_TE 0x010 /* R,8 */ +#define OSTM_TS 0x014 /* W,8 */ +#define OSTM_TT 0x018 /* W,8 */ +#define OSTM_CTL 0x020 /* RW,8 */ + +#define TE 0x01 +#define TS 0x01 +#define TT 0x01 +#define CTL_PERIODIC 0x00 +#define CTL_ONESHOT 0x02 +#define CTL_FREERUN 0x02 + +static struct ostm_device *ced_to_ostm(struct clock_event_device *ced) +{ + return container_of(ced, struct ostm_device, ced); +} + +static int __init ostm_init_clksrc(struct ostm_device *ostm) +{ + int ret; + + /* irq not used (clock sources don't use interrupts) */ + + /* stop counter */ + iowrite8(TT, ostm->base + OSTM_TT); + while (ioread8(ostm->base + OSTM_TE) & TE) + ; + + /* setup as freerun */ + iowrite32(0, ostm->base + OSTM_CMP); + iowrite8(CTL_FREERUN, ostm->base + OSTM_CTL); + iowrite8(TS, ostm->base + OSTM_TS); + + /* register */ + ret = clocksource_mmio_init(ostm->base + OSTM_CNT, + "ostm", ostm->rate, + 300, 32, clocksource_mmio_readl_up); + + return ret; +} + +static u64 notrace ostm_read_sched_clock(void) +{ + return ioread32(system_clock); +} + +static int __init ostm_init_sched_clock(struct ostm_device *ostm) +{ + unsigned long flags; + + system_clock = ostm->base + OSTM_CNT; + local_irq_save(flags); + local_irq_disable(); + sched_clock_register(ostm_read_sched_clock, 32, ostm->rate); + local_irq_restore(flags); + + return 0; +} + +static void ostm_clkevt_timer_stop(struct ostm_device *ostm) +{ + if (ioread8(ostm->base + OSTM_TE) & TE) { + iowrite8(TT, ostm->base + OSTM_TT); + while (ioread8(ostm->base + OSTM_TE) & TE) + ; + } +} + +static int ostm_clock_event_next(unsigned long delta, + struct clock_event_device *ced) +{ + struct ostm_device *ostm = ced_to_ostm(ced); + + WARN_ON(!clockevent_state_oneshot(ced)); + + ostm_clkevt_timer_stop(ostm); + + iowrite32(delta, ostm->base + OSTM_CMP); + iowrite8(CTL_ONESHOT, ostm->base + OSTM_CTL); + iowrite8(TS, ostm->base + OSTM_TS); + + return 0; +} + +static int ostm_shutdown(struct clock_event_device *ced) +{ + struct ostm_device *ostm = ced_to_ostm(ced); + + ostm_clkevt_timer_stop(ostm); + + return 0; +} +static int ostm_set_periodic(struct clock_event_device *ced) +{ + struct ostm_device *ostm = ced_to_ostm(ced); + + if (clockevent_state_oneshot(ced) || clockevent_state_periodic(ced)) + ostm_clkevt_timer_stop(ostm); + + iowrite32(ostm->ticks_per_jiffy - 1, ostm->base + OSTM_CMP); + iowrite8(CTL_PERIODIC, ostm->base + OSTM_CTL); + iowrite8(TS, ostm->base + OSTM_TS); + + return 0; +} + +static int ostm_set_oneshot(struct clock_event_device *ced) +{ + struct ostm_device *ostm = ced_to_ostm(ced); + + ostm_clkevt_timer_stop(ostm); + + return 0; +} + +static irqreturn_t ostm_timer_interrupt(int irq, void *dev_id) +{ + struct ostm_device *ostm = dev_id; + + if (clockevent_state_oneshot(&ostm->ced)) + ostm_clkevt_timer_stop(ostm); + + /* notify clockevent layer */ + if (ostm->ced.event_handler) + ostm->ced.event_handler(&ostm->ced); + + return IRQ_HANDLED; +} + +static int __init ostm_init_clkevt(struct ostm_device *ostm) +{ + struct clock_event_device *ced = &ostm->ced; + int ret = -ENXIO; + + ret = request_irq(ostm->irq, ostm_timer_interrupt, + IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING, + dev_name(&ostm->pdev->dev), ostm); + if (ret) { + dev_err(&ostm->pdev->dev, "failed to request irq\n"); + return ret; + } + + ced->name = "ostm"; + ced->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC; + ced->set_state_shutdown = ostm_shutdown; + ced->set_state_periodic = ostm_set_periodic; + ced->set_state_oneshot = ostm_set_oneshot; + ced->set_next_event = ostm_clock_event_next; + ced->shift = 32; + ced->rating = 300; + ced->cpumask = cpumask_of(0); + clockevents_config_and_register(ced, ostm->rate, 0xf, 0xffffffff); + + return 0; +} + +static int __init ostm_probe(struct platform_device *pdev) +{ + struct ostm_device *ostm; + struct resource *res; + int ret = -EFAULT; + + if (!is_early_platform_device(pdev)) { + pm_runtime_set_active(&pdev->dev); + pm_runtime_enable(&pdev->dev); + } + + ostm = platform_get_drvdata(pdev); + if (ostm) { + dev_info(&pdev->dev, "kept as earlytimer\n"); + ret = 0; + goto out; + } + + ostm = kzalloc(sizeof(*ostm), GFP_KERNEL); + if (!ostm) { + dev_err(&pdev->dev, "failed to allocate memory\n"); + return -ENOMEM; + } + + ostm->pdev = pdev; + platform_set_drvdata(ostm->pdev, ostm); + + res = platform_get_resource(ostm->pdev, IORESOURCE_MEM, 0); + if (!res) { + dev_err(&ostm->pdev->dev, "failed to get I/O memory\n"); + goto err; + } + + ostm->base = ioremap_nocache(res->start, resource_size(res)); + if (!ostm->base) { + dev_err(&ostm->pdev->dev, "failed to remap I/O memory\n"); + goto err; + } + + ostm->irq = platform_get_irq(ostm->pdev, 0); + if (ostm->irq < 0) { + dev_err(&ostm->pdev->dev, "failed to get irq\n"); + goto err; + } + + ostm->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(ostm->clk)) { + dev_err(&ostm->pdev->dev, "failed to get clock\n"); + ostm->clk = NULL; + goto err; + } + + ret = clk_prepare_enable(ostm->clk); + if (ret) { + dev_err(&ostm->pdev->dev, "failed to enable clock\n"); + goto err; + } + + ostm->rate = clk_get_rate(ostm->clk); + ostm->ticks_per_jiffy = (ostm->rate + HZ / 2) / HZ; + + /* First probed device will be used as system clocksource */ + if (!system_clock) { + /* use as clocksource */ + ret = ostm_init_clksrc(ostm); + + /* use as system scheduling clock */ + if (!ret) + ret = ostm_init_sched_clock(ostm); + + if (ret) { + dev_err(&pdev->dev, "failed to use as sched_clock\n"); + system_clock = (void *)-1; /* prevent future attempts */ + ret = 0; /* still works as clocksource */ + } + + if (!ret) + dev_info(&pdev->dev, "used for clocksource\n"); + } else { + /* use as clock event */ + ret = ostm_init_clkevt(ostm); + + if (!ret) + dev_info(&pdev->dev, "used for clock events\n"); + } + +err: + if (ret) { + if (ostm->clk) + clk_disable_unprepare(ostm->clk); + if (ostm->base) + iounmap(ostm->base); + kfree(ostm); + platform_set_drvdata(pdev, NULL); + pm_runtime_idle(&pdev->dev); + return ret; + } + + if (is_early_platform_device(pdev)) + return ret; + +out: + pm_runtime_irq_safe(&pdev->dev); + + return ret; +} + +static int ostm_remove(struct platform_device *pdev) +{ + return -EBUSY; /* cannot unregister clockevent */ +} + +static const struct of_device_id ostm_of_table[] __maybe_unused = { + { .compatible = "renesas,ostm" }, + { } +}; +MODULE_DEVICE_TABLE(of, ostm_of_table); + +static struct platform_driver ostm_timer = { + .probe = ostm_probe, + .remove = ostm_remove, + .driver = { + .name = "ostm", + .of_match_table = of_match_ptr(ostm_of_table), + }, +}; + +static int __init ostm_init(void) +{ + return platform_driver_register(&ostm_timer); +} + +static void __exit ostm_exit(void) +{ + platform_driver_unregister(&ostm_timer); +} + +early_platform_init("earlytimer", &ostm_timer); +subsys_initcall(ostm_init); +module_exit(ostm_exit); + +MODULE_AUTHOR("Chris Brandt"); +MODULE_DESCRIPTION("Renesas OSTM Timer Driver"); +MODULE_LICENSE("GPL v2");
This patch adds a OSTM driver for the Renesas architecture. Signed-off-by: Chris Brandt <chris.brandt@renesas.com> --- v2: * changed implementation to be independent channel nodes --- arch/arm/mach-shmobile/Kconfig | 1 + drivers/clocksource/Kconfig | 12 ++ drivers/clocksource/Makefile | 1 + drivers/clocksource/renesas-ostm.c | 349 +++++++++++++++++++++++++++++++++++++ 4 files changed, 363 insertions(+) create mode 100644 drivers/clocksource/renesas-ostm.c