* [PATCH 1/3] clocksource: move PXA timer to clocksource framework @ 2014-06-21 22:09 ` Robert Jarzmik 0 siblings, 0 replies; 20+ messages in thread From: Robert Jarzmik @ 2014-06-21 22:09 UTC (permalink / raw) To: Haojian Zhuang, Daniel Lezcano, Thomas Gleixner Cc: linux-arm-kernel, linux-kernel, Robert Jarzmik Move time.c from arch/arm/mach-pxa/time.c to drivers/clocksource/pxa_timer.c. Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> --- arch/arm/mach-pxa/Makefile | 2 +- drivers/clocksource/Makefile | 1 + arch/arm/mach-pxa/time.c => drivers/clocksource/pxa_timer.c | 0 3 files changed, 2 insertions(+), 1 deletion(-) rename arch/arm/mach-pxa/time.c => drivers/clocksource/pxa_timer.c (100%) diff --git a/arch/arm/mach-pxa/Makefile b/arch/arm/mach-pxa/Makefile index 648867a..2fe1824 100644 --- a/arch/arm/mach-pxa/Makefile +++ b/arch/arm/mach-pxa/Makefile @@ -4,7 +4,7 @@ # Common support (must be linked before board specific support) obj-y += clock.o devices.o generic.o irq.o \ - time.o reset.o + reset.o obj-$(CONFIG_PM) += pm.o sleep.o standby.o # Generic drivers that other drivers may depend upon diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile index 98cb6c5..e224125 100644 --- a/drivers/clocksource/Makefile +++ b/drivers/clocksource/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_ARCH_BCM2835) += bcm2835_timer.o obj-$(CONFIG_ARCH_MARCO) += timer-marco.o obj-$(CONFIG_ARCH_MOXART) += moxart_timer.o obj-$(CONFIG_ARCH_MXS) += mxs_timer.o +obj-$(CONFIG_ARCH_PXA) += pxa_timer.o obj-$(CONFIG_ARCH_PRIMA2) += timer-prima2.o obj-$(CONFIG_ARCH_U300) += timer-u300.o obj-$(CONFIG_SUN4I_TIMER) += sun4i_timer.o diff --git a/arch/arm/mach-pxa/time.c b/drivers/clocksource/pxa_timer.c similarity index 100% rename from arch/arm/mach-pxa/time.c rename to drivers/clocksource/pxa_timer.c -- 2.0.0.rc2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/3] clocksource: move PXA timer to clocksource framework @ 2014-06-21 22:09 ` Robert Jarzmik 0 siblings, 0 replies; 20+ messages in thread From: Robert Jarzmik @ 2014-06-21 22:09 UTC (permalink / raw) To: linux-arm-kernel Move time.c from arch/arm/mach-pxa/time.c to drivers/clocksource/pxa_timer.c. Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> --- arch/arm/mach-pxa/Makefile | 2 +- drivers/clocksource/Makefile | 1 + arch/arm/mach-pxa/time.c => drivers/clocksource/pxa_timer.c | 0 3 files changed, 2 insertions(+), 1 deletion(-) rename arch/arm/mach-pxa/time.c => drivers/clocksource/pxa_timer.c (100%) diff --git a/arch/arm/mach-pxa/Makefile b/arch/arm/mach-pxa/Makefile index 648867a..2fe1824 100644 --- a/arch/arm/mach-pxa/Makefile +++ b/arch/arm/mach-pxa/Makefile @@ -4,7 +4,7 @@ # Common support (must be linked before board specific support) obj-y += clock.o devices.o generic.o irq.o \ - time.o reset.o + reset.o obj-$(CONFIG_PM) += pm.o sleep.o standby.o # Generic drivers that other drivers may depend upon diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile index 98cb6c5..e224125 100644 --- a/drivers/clocksource/Makefile +++ b/drivers/clocksource/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_ARCH_BCM2835) += bcm2835_timer.o obj-$(CONFIG_ARCH_MARCO) += timer-marco.o obj-$(CONFIG_ARCH_MOXART) += moxart_timer.o obj-$(CONFIG_ARCH_MXS) += mxs_timer.o +obj-$(CONFIG_ARCH_PXA) += pxa_timer.o obj-$(CONFIG_ARCH_PRIMA2) += timer-prima2.o obj-$(CONFIG_ARCH_U300) += timer-u300.o obj-$(CONFIG_SUN4I_TIMER) += sun4i_timer.o diff --git a/arch/arm/mach-pxa/time.c b/drivers/clocksource/pxa_timer.c similarity index 100% rename from arch/arm/mach-pxa/time.c rename to drivers/clocksource/pxa_timer.c -- 2.0.0.rc2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/3] clocksource: add device-tree support for PXA timer 2014-06-21 22:09 ` Robert Jarzmik @ 2014-06-21 22:09 ` Robert Jarzmik -1 siblings, 0 replies; 20+ messages in thread From: Robert Jarzmik @ 2014-06-21 22:09 UTC (permalink / raw) To: Haojian Zhuang, Daniel Lezcano, Thomas Gleixner Cc: linux-arm-kernel, linux-kernel, Robert Jarzmik Add device-tree support to PXA platforms. Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> --- drivers/clocksource/pxa_timer.c | 131 ++++++++++++++++++++++++++++++---------- 1 file changed, 98 insertions(+), 33 deletions(-) diff --git a/drivers/clocksource/pxa_timer.c b/drivers/clocksource/pxa_timer.c index fca174e..67da3f5 100644 --- a/drivers/clocksource/pxa_timer.c +++ b/drivers/clocksource/pxa_timer.c @@ -15,14 +15,41 @@ #include <linux/kernel.h> #include <linux/init.h> #include <linux/interrupt.h> +#include <linux/clk.h> #include <linux/clockchips.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> #include <linux/sched_clock.h> #include <asm/div64.h> #include <asm/mach/irq.h> #include <asm/mach/time.h> -#include <mach/regs-ost.h> #include <mach/irqs.h> +#include <mach/hardware.h> + +#define OSMR0 0x00 /* */ +#define OSMR1 0x04 /* */ +#define OSMR2 0x08 /* */ +#define OSMR3 0x0C /* */ +#define OSMR4 0x80 /* */ +#define OSCR 0x10 /* OS Timer Counter Register */ +#define OSCR4 0x40 /* OS Timer Counter Register */ +#define OMCR4 0xC0 /* */ +#define OSSR 0x14 /* OS Timer Status Register */ +#define OWER 0x18 /* OS Timer Watchdog Enable Register */ +#define OIER 0x1C /* OS Timer Interrupt Enable Register */ + +#define OSSR_M3 (1 << 3) /* Match status channel 3 */ +#define OSSR_M2 (1 << 2) /* Match status channel 2 */ +#define OSSR_M1 (1 << 1) /* Match status channel 1 */ +#define OSSR_M0 (1 << 0) /* Match status channel 0 */ + +#define OWER_WME (1 << 0) /* Watchdog Match Enable */ + +#define OIER_E3 (1 << 3) /* Interrupt enable channel 3 */ +#define OIER_E2 (1 << 2) /* Interrupt enable channel 2 */ +#define OIER_E1 (1 << 1) /* Interrupt enable channel 1 */ +#define OIER_E0 (1 << 0) /* Interrupt enable channel 0 */ /* * This is PXA's sched_clock implementation. This has a resolution @@ -33,9 +60,14 @@ * calls to sched_clock() which should always be the case in practice. */ +#define timer_readl(reg) readl_relaxed(timer_base + (reg)) +#define timer_writel(val, reg) writel_relaxed((val), timer_base + (reg)) + +static void __iomem *timer_base; + static u64 notrace pxa_read_sched_clock(void) { - return readl_relaxed(OSCR); + return timer_readl(OSCR); } @@ -47,8 +79,8 @@ pxa_ost0_interrupt(int irq, void *dev_id) struct clock_event_device *c = dev_id; /* Disarm the compare/match, signal the event. */ - writel_relaxed(readl_relaxed(OIER) & ~OIER_E0, OIER); - writel_relaxed(OSSR_M0, OSSR); + timer_writel(timer_readl(OIER) & ~OIER_E0, OIER); + timer_writel(OSSR_M0, OSSR); c->event_handler(c); return IRQ_HANDLED; @@ -59,10 +91,10 @@ pxa_osmr0_set_next_event(unsigned long delta, struct clock_event_device *dev) { unsigned long next, oscr; - writel_relaxed(readl_relaxed(OIER) | OIER_E0, OIER); - next = readl_relaxed(OSCR) + delta; - writel_relaxed(next, OSMR0); - oscr = readl_relaxed(OSCR); + timer_writel(timer_readl(OIER) | OIER_E0, OIER); + next = timer_readl(OSCR) + delta; + timer_writel(next, OSMR0); + oscr = timer_readl(OSCR); return (signed)(next - oscr) <= MIN_OSCR_DELTA ? -ETIME : 0; } @@ -72,15 +104,15 @@ pxa_osmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *dev) { switch (mode) { case CLOCK_EVT_MODE_ONESHOT: - writel_relaxed(readl_relaxed(OIER) & ~OIER_E0, OIER); - writel_relaxed(OSSR_M0, OSSR); + timer_writel(timer_readl(OIER) & ~OIER_E0, OIER); + timer_writel(OSSR_M0, OSSR); break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: /* initializing, released, or preparing for suspend */ - writel_relaxed(readl_relaxed(OIER) & ~OIER_E0, OIER); - writel_relaxed(OSSR_M0, OSSR); + timer_writel(timer_readl(OIER) & ~OIER_E0, OIER); + timer_writel(OSSR_M0, OSSR); break; case CLOCK_EVT_MODE_RESUME: @@ -94,12 +126,12 @@ static unsigned long osmr[4], oier, oscr; static void pxa_timer_suspend(struct clock_event_device *cedev) { - osmr[0] = readl_relaxed(OSMR0); - osmr[1] = readl_relaxed(OSMR1); - osmr[2] = readl_relaxed(OSMR2); - osmr[3] = readl_relaxed(OSMR3); - oier = readl_relaxed(OIER); - oscr = readl_relaxed(OSCR); + osmr[0] = timer_readl(OSMR0); + osmr[1] = timer_readl(OSMR1); + osmr[2] = timer_readl(OSMR2); + osmr[3] = timer_readl(OSMR3); + oier = timer_readl(OIER); + oscr = timer_readl(OSCR); } static void pxa_timer_resume(struct clock_event_device *cedev) @@ -113,12 +145,12 @@ static void pxa_timer_resume(struct clock_event_device *cedev) if (osmr[0] - oscr < MIN_OSCR_DELTA) osmr[0] += MIN_OSCR_DELTA; - writel_relaxed(osmr[0], OSMR0); - writel_relaxed(osmr[1], OSMR1); - writel_relaxed(osmr[2], OSMR2); - writel_relaxed(osmr[3], OSMR3); - writel_relaxed(oier, OIER); - writel_relaxed(oscr, OSCR); + timer_writel(osmr[0], OSMR0); + timer_writel(osmr[1], OSMR1); + timer_writel(osmr[2], OSMR2); + timer_writel(osmr[3], OSMR3); + timer_writel(oier, OIER); + timer_writel(oscr, OSCR); } #else #define pxa_timer_suspend NULL @@ -142,21 +174,54 @@ static struct irqaction pxa_ost0_irq = { .dev_id = &ckevt_pxa_osmr0, }; -void __init pxa_timer_init(void) +static void pxa_timer_common_init(int irq, unsigned long clock_tick_rate) { - unsigned long clock_tick_rate = get_clock_tick_rate(); - - writel_relaxed(0, OIER); - writel_relaxed(OSSR_M0 | OSSR_M1 | OSSR_M2 | OSSR_M3, OSSR); + timer_writel(0, OIER); + timer_writel(OSSR_M0 | OSSR_M1 | OSSR_M2 | OSSR_M3, OSSR); sched_clock_register(pxa_read_sched_clock, 32, clock_tick_rate); ckevt_pxa_osmr0.cpumask = cpumask_of(0); - setup_irq(IRQ_OST0, &pxa_ost0_irq); + setup_irq(irq, &pxa_ost0_irq); - clocksource_mmio_init(OSCR, "oscr0", clock_tick_rate, 200, 32, - clocksource_mmio_readl_up); + clocksource_mmio_init(timer_base + OSCR, "oscr0", clock_tick_rate, 200, + 32, clocksource_mmio_readl_up); clockevents_config_and_register(&ckevt_pxa_osmr0, clock_tick_rate, - MIN_OSCR_DELTA * 2, 0x7fffffff); + MIN_OSCR_DELTA * 2, 0x7fffffff); +} + +static void __init pxa_timer_dt_init(struct device_node *np) +{ + struct clk *clk; + int irq; + + /* timer registers are shared with watchdog timer */ + timer_base = of_iomap(np, 0); + if (!timer_base) + panic("%s: unable to map resource\n", np->name); + + clk = of_clk_get(np, 0); + if (IS_ERR(clk)) + panic("%s: unable to get clk\n", np->name); + clk_prepare_enable(clk); + + /* we are only interested in OS-timer0 irq */ + irq = irq_of_parse_and_map(np, 0); + if (irq <= 0) + panic("%s: unable to parse OS-timer0 irq\n", np->name); + + pxa_timer_common_init(irq, clk_get_rate(clk)); +} +CLOCKSOURCE_OF_DECLARE(pxa_timer, "marvell,pxa-timer", pxa_timer_dt_init); + +/* + * Legacy timer init for non device-tree boards. + */ +void __init pxa_timer_init(void) +{ + unsigned long clock_tick_rate = get_clock_tick_rate(); + + timer_base = io_p2v(0x40a00000); + pxa_timer_common_init(IRQ_OST0, clock_tick_rate); } -- 2.0.0.rc2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/3] clocksource: add device-tree support for PXA timer @ 2014-06-21 22:09 ` Robert Jarzmik 0 siblings, 0 replies; 20+ messages in thread From: Robert Jarzmik @ 2014-06-21 22:09 UTC (permalink / raw) To: linux-arm-kernel Add device-tree support to PXA platforms. Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> --- drivers/clocksource/pxa_timer.c | 131 ++++++++++++++++++++++++++++++---------- 1 file changed, 98 insertions(+), 33 deletions(-) diff --git a/drivers/clocksource/pxa_timer.c b/drivers/clocksource/pxa_timer.c index fca174e..67da3f5 100644 --- a/drivers/clocksource/pxa_timer.c +++ b/drivers/clocksource/pxa_timer.c @@ -15,14 +15,41 @@ #include <linux/kernel.h> #include <linux/init.h> #include <linux/interrupt.h> +#include <linux/clk.h> #include <linux/clockchips.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> #include <linux/sched_clock.h> #include <asm/div64.h> #include <asm/mach/irq.h> #include <asm/mach/time.h> -#include <mach/regs-ost.h> #include <mach/irqs.h> +#include <mach/hardware.h> + +#define OSMR0 0x00 /* */ +#define OSMR1 0x04 /* */ +#define OSMR2 0x08 /* */ +#define OSMR3 0x0C /* */ +#define OSMR4 0x80 /* */ +#define OSCR 0x10 /* OS Timer Counter Register */ +#define OSCR4 0x40 /* OS Timer Counter Register */ +#define OMCR4 0xC0 /* */ +#define OSSR 0x14 /* OS Timer Status Register */ +#define OWER 0x18 /* OS Timer Watchdog Enable Register */ +#define OIER 0x1C /* OS Timer Interrupt Enable Register */ + +#define OSSR_M3 (1 << 3) /* Match status channel 3 */ +#define OSSR_M2 (1 << 2) /* Match status channel 2 */ +#define OSSR_M1 (1 << 1) /* Match status channel 1 */ +#define OSSR_M0 (1 << 0) /* Match status channel 0 */ + +#define OWER_WME (1 << 0) /* Watchdog Match Enable */ + +#define OIER_E3 (1 << 3) /* Interrupt enable channel 3 */ +#define OIER_E2 (1 << 2) /* Interrupt enable channel 2 */ +#define OIER_E1 (1 << 1) /* Interrupt enable channel 1 */ +#define OIER_E0 (1 << 0) /* Interrupt enable channel 0 */ /* * This is PXA's sched_clock implementation. This has a resolution @@ -33,9 +60,14 @@ * calls to sched_clock() which should always be the case in practice. */ +#define timer_readl(reg) readl_relaxed(timer_base + (reg)) +#define timer_writel(val, reg) writel_relaxed((val), timer_base + (reg)) + +static void __iomem *timer_base; + static u64 notrace pxa_read_sched_clock(void) { - return readl_relaxed(OSCR); + return timer_readl(OSCR); } @@ -47,8 +79,8 @@ pxa_ost0_interrupt(int irq, void *dev_id) struct clock_event_device *c = dev_id; /* Disarm the compare/match, signal the event. */ - writel_relaxed(readl_relaxed(OIER) & ~OIER_E0, OIER); - writel_relaxed(OSSR_M0, OSSR); + timer_writel(timer_readl(OIER) & ~OIER_E0, OIER); + timer_writel(OSSR_M0, OSSR); c->event_handler(c); return IRQ_HANDLED; @@ -59,10 +91,10 @@ pxa_osmr0_set_next_event(unsigned long delta, struct clock_event_device *dev) { unsigned long next, oscr; - writel_relaxed(readl_relaxed(OIER) | OIER_E0, OIER); - next = readl_relaxed(OSCR) + delta; - writel_relaxed(next, OSMR0); - oscr = readl_relaxed(OSCR); + timer_writel(timer_readl(OIER) | OIER_E0, OIER); + next = timer_readl(OSCR) + delta; + timer_writel(next, OSMR0); + oscr = timer_readl(OSCR); return (signed)(next - oscr) <= MIN_OSCR_DELTA ? -ETIME : 0; } @@ -72,15 +104,15 @@ pxa_osmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *dev) { switch (mode) { case CLOCK_EVT_MODE_ONESHOT: - writel_relaxed(readl_relaxed(OIER) & ~OIER_E0, OIER); - writel_relaxed(OSSR_M0, OSSR); + timer_writel(timer_readl(OIER) & ~OIER_E0, OIER); + timer_writel(OSSR_M0, OSSR); break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: /* initializing, released, or preparing for suspend */ - writel_relaxed(readl_relaxed(OIER) & ~OIER_E0, OIER); - writel_relaxed(OSSR_M0, OSSR); + timer_writel(timer_readl(OIER) & ~OIER_E0, OIER); + timer_writel(OSSR_M0, OSSR); break; case CLOCK_EVT_MODE_RESUME: @@ -94,12 +126,12 @@ static unsigned long osmr[4], oier, oscr; static void pxa_timer_suspend(struct clock_event_device *cedev) { - osmr[0] = readl_relaxed(OSMR0); - osmr[1] = readl_relaxed(OSMR1); - osmr[2] = readl_relaxed(OSMR2); - osmr[3] = readl_relaxed(OSMR3); - oier = readl_relaxed(OIER); - oscr = readl_relaxed(OSCR); + osmr[0] = timer_readl(OSMR0); + osmr[1] = timer_readl(OSMR1); + osmr[2] = timer_readl(OSMR2); + osmr[3] = timer_readl(OSMR3); + oier = timer_readl(OIER); + oscr = timer_readl(OSCR); } static void pxa_timer_resume(struct clock_event_device *cedev) @@ -113,12 +145,12 @@ static void pxa_timer_resume(struct clock_event_device *cedev) if (osmr[0] - oscr < MIN_OSCR_DELTA) osmr[0] += MIN_OSCR_DELTA; - writel_relaxed(osmr[0], OSMR0); - writel_relaxed(osmr[1], OSMR1); - writel_relaxed(osmr[2], OSMR2); - writel_relaxed(osmr[3], OSMR3); - writel_relaxed(oier, OIER); - writel_relaxed(oscr, OSCR); + timer_writel(osmr[0], OSMR0); + timer_writel(osmr[1], OSMR1); + timer_writel(osmr[2], OSMR2); + timer_writel(osmr[3], OSMR3); + timer_writel(oier, OIER); + timer_writel(oscr, OSCR); } #else #define pxa_timer_suspend NULL @@ -142,21 +174,54 @@ static struct irqaction pxa_ost0_irq = { .dev_id = &ckevt_pxa_osmr0, }; -void __init pxa_timer_init(void) +static void pxa_timer_common_init(int irq, unsigned long clock_tick_rate) { - unsigned long clock_tick_rate = get_clock_tick_rate(); - - writel_relaxed(0, OIER); - writel_relaxed(OSSR_M0 | OSSR_M1 | OSSR_M2 | OSSR_M3, OSSR); + timer_writel(0, OIER); + timer_writel(OSSR_M0 | OSSR_M1 | OSSR_M2 | OSSR_M3, OSSR); sched_clock_register(pxa_read_sched_clock, 32, clock_tick_rate); ckevt_pxa_osmr0.cpumask = cpumask_of(0); - setup_irq(IRQ_OST0, &pxa_ost0_irq); + setup_irq(irq, &pxa_ost0_irq); - clocksource_mmio_init(OSCR, "oscr0", clock_tick_rate, 200, 32, - clocksource_mmio_readl_up); + clocksource_mmio_init(timer_base + OSCR, "oscr0", clock_tick_rate, 200, + 32, clocksource_mmio_readl_up); clockevents_config_and_register(&ckevt_pxa_osmr0, clock_tick_rate, - MIN_OSCR_DELTA * 2, 0x7fffffff); + MIN_OSCR_DELTA * 2, 0x7fffffff); +} + +static void __init pxa_timer_dt_init(struct device_node *np) +{ + struct clk *clk; + int irq; + + /* timer registers are shared with watchdog timer */ + timer_base = of_iomap(np, 0); + if (!timer_base) + panic("%s: unable to map resource\n", np->name); + + clk = of_clk_get(np, 0); + if (IS_ERR(clk)) + panic("%s: unable to get clk\n", np->name); + clk_prepare_enable(clk); + + /* we are only interested in OS-timer0 irq */ + irq = irq_of_parse_and_map(np, 0); + if (irq <= 0) + panic("%s: unable to parse OS-timer0 irq\n", np->name); + + pxa_timer_common_init(irq, clk_get_rate(clk)); +} +CLOCKSOURCE_OF_DECLARE(pxa_timer, "marvell,pxa-timer", pxa_timer_dt_init); + +/* + * Legacy timer init for non device-tree boards. + */ +void __init pxa_timer_init(void) +{ + unsigned long clock_tick_rate = get_clock_tick_rate(); + + timer_base = io_p2v(0x40a00000); + pxa_timer_common_init(IRQ_OST0, clock_tick_rate); } -- 2.0.0.rc2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] clocksource: add device-tree support for PXA timer 2014-06-21 22:09 ` Robert Jarzmik @ 2014-07-03 12:05 ` Daniel Lezcano -1 siblings, 0 replies; 20+ messages in thread From: Daniel Lezcano @ 2014-07-03 12:05 UTC (permalink / raw) To: Robert Jarzmik, Haojian Zhuang, Thomas Gleixner Cc: linux-arm-kernel, linux-kernel On 06/22/2014 12:09 AM, Robert Jarzmik wrote: > Add device-tree support to PXA platforms. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > --- > drivers/clocksource/pxa_timer.c | 131 ++++++++++++++++++++++++++++++---------- > 1 file changed, 98 insertions(+), 33 deletions(-) > > diff --git a/drivers/clocksource/pxa_timer.c b/drivers/clocksource/pxa_timer.c > index fca174e..67da3f5 100644 > --- a/drivers/clocksource/pxa_timer.c > +++ b/drivers/clocksource/pxa_timer.c > @@ -15,14 +15,41 @@ > #include <linux/kernel.h> > #include <linux/init.h> > #include <linux/interrupt.h> > +#include <linux/clk.h> > #include <linux/clockchips.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > #include <linux/sched_clock.h> > > #include <asm/div64.h> > #include <asm/mach/irq.h> > #include <asm/mach/time.h> > -#include <mach/regs-ost.h> > #include <mach/irqs.h> > +#include <mach/hardware.h> Now as the driver is in 'drivers', do not reference the headers files in mach. Moving the driver to the drivers directory implies some cleanup with the headers dependencies. > +#define OSMR0 0x00 /* */ > +#define OSMR1 0x04 /* */ > +#define OSMR2 0x08 /* */ > +#define OSMR3 0x0C /* */ > +#define OSMR4 0x80 /* */ Can you please remove those unused empty comment or fill them with something appropriate. > +#define OSCR 0x10 /* OS Timer Counter Register */ > +#define OSCR4 0x40 /* OS Timer Counter Register */ > +#define OMCR4 0xC0 /* */ > +#define OSSR 0x14 /* OS Timer Status Register */ > +#define OWER 0x18 /* OS Timer Watchdog Enable Register */ > +#define OIER 0x1C /* OS Timer Interrupt Enable Register */ > + > +#define OSSR_M3 (1 << 3) /* Match status channel 3 */ > +#define OSSR_M2 (1 << 2) /* Match status channel 2 */ > +#define OSSR_M1 (1 << 1) /* Match status channel 1 */ > +#define OSSR_M0 (1 << 0) /* Match status channel 0 */ > + > +#define OWER_WME (1 << 0) /* Watchdog Match Enable */ > + > +#define OIER_E3 (1 << 3) /* Interrupt enable channel 3 */ > +#define OIER_E2 (1 << 2) /* Interrupt enable channel 2 */ > +#define OIER_E1 (1 << 1) /* Interrupt enable channel 1 */ > +#define OIER_E0 (1 << 0) /* Interrupt enable channel 0 */ Is it possible to do some cleanups around regs-ost.h and here in order to remove the unused macros. Also, it seems some define will be duplicate as they are shared with the watchdog. Any plan to fix that ? > /* > * This is PXA's sched_clock implementation. This has a resolution > @@ -33,9 +60,14 @@ > * calls to sched_clock() which should always be the case in practice. > */ > > +#define timer_readl(reg) readl_relaxed(timer_base + (reg)) > +#define timer_writel(val, reg) writel_relaxed((val), timer_base + (reg)) > + > +static void __iomem *timer_base; > + > static u64 notrace pxa_read_sched_clock(void) > { > - return readl_relaxed(OSCR); So here there is a change which is not explained in the changelog (timer_base offset). Even it is obvious for me because I am used to see this kind of code, that would deserve a better description in the changelog. > + return timer_readl(OSCR); > } > [ ... ] > +static void __init pxa_timer_dt_init(struct device_node *np) > +{ > + struct clk *clk; > + int irq; > + > + /* timer registers are shared with watchdog timer */ > + timer_base = of_iomap(np, 0); > + if (!timer_base) > + panic("%s: unable to map resource\n", np->name); > + > + clk = of_clk_get(np, 0); > + if (IS_ERR(clk)) > + panic("%s: unable to get clk\n", np->name); > + clk_prepare_enable(clk); > + > + /* we are only interested in OS-timer0 irq */ > + irq = irq_of_parse_and_map(np, 0); > + if (irq <= 0) > + panic("%s: unable to parse OS-timer0 irq\n", np->name); Is the 'panic' desirable ? The clksrc-of is written in a way to use different clocks, no ? > + > + pxa_timer_common_init(irq, clk_get_rate(clk)); > +} > +CLOCKSOURCE_OF_DECLARE(pxa_timer, "marvell,pxa-timer", pxa_timer_dt_init); > + > +/* > + * Legacy timer init for non device-tree boards. > + */ > +void __init pxa_timer_init(void) > +{ > + unsigned long clock_tick_rate = get_clock_tick_rate(); > + > + timer_base = io_p2v(0x40a00000); > + pxa_timer_common_init(IRQ_OST0, clock_tick_rate); > } > -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/3] clocksource: add device-tree support for PXA timer @ 2014-07-03 12:05 ` Daniel Lezcano 0 siblings, 0 replies; 20+ messages in thread From: Daniel Lezcano @ 2014-07-03 12:05 UTC (permalink / raw) To: linux-arm-kernel On 06/22/2014 12:09 AM, Robert Jarzmik wrote: > Add device-tree support to PXA platforms. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > --- > drivers/clocksource/pxa_timer.c | 131 ++++++++++++++++++++++++++++++---------- > 1 file changed, 98 insertions(+), 33 deletions(-) > > diff --git a/drivers/clocksource/pxa_timer.c b/drivers/clocksource/pxa_timer.c > index fca174e..67da3f5 100644 > --- a/drivers/clocksource/pxa_timer.c > +++ b/drivers/clocksource/pxa_timer.c > @@ -15,14 +15,41 @@ > #include <linux/kernel.h> > #include <linux/init.h> > #include <linux/interrupt.h> > +#include <linux/clk.h> > #include <linux/clockchips.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > #include <linux/sched_clock.h> > > #include <asm/div64.h> > #include <asm/mach/irq.h> > #include <asm/mach/time.h> > -#include <mach/regs-ost.h> > #include <mach/irqs.h> > +#include <mach/hardware.h> Now as the driver is in 'drivers', do not reference the headers files in mach. Moving the driver to the drivers directory implies some cleanup with the headers dependencies. > +#define OSMR0 0x00 /* */ > +#define OSMR1 0x04 /* */ > +#define OSMR2 0x08 /* */ > +#define OSMR3 0x0C /* */ > +#define OSMR4 0x80 /* */ Can you please remove those unused empty comment or fill them with something appropriate. > +#define OSCR 0x10 /* OS Timer Counter Register */ > +#define OSCR4 0x40 /* OS Timer Counter Register */ > +#define OMCR4 0xC0 /* */ > +#define OSSR 0x14 /* OS Timer Status Register */ > +#define OWER 0x18 /* OS Timer Watchdog Enable Register */ > +#define OIER 0x1C /* OS Timer Interrupt Enable Register */ > + > +#define OSSR_M3 (1 << 3) /* Match status channel 3 */ > +#define OSSR_M2 (1 << 2) /* Match status channel 2 */ > +#define OSSR_M1 (1 << 1) /* Match status channel 1 */ > +#define OSSR_M0 (1 << 0) /* Match status channel 0 */ > + > +#define OWER_WME (1 << 0) /* Watchdog Match Enable */ > + > +#define OIER_E3 (1 << 3) /* Interrupt enable channel 3 */ > +#define OIER_E2 (1 << 2) /* Interrupt enable channel 2 */ > +#define OIER_E1 (1 << 1) /* Interrupt enable channel 1 */ > +#define OIER_E0 (1 << 0) /* Interrupt enable channel 0 */ Is it possible to do some cleanups around regs-ost.h and here in order to remove the unused macros. Also, it seems some define will be duplicate as they are shared with the watchdog. Any plan to fix that ? > /* > * This is PXA's sched_clock implementation. This has a resolution > @@ -33,9 +60,14 @@ > * calls to sched_clock() which should always be the case in practice. > */ > > +#define timer_readl(reg) readl_relaxed(timer_base + (reg)) > +#define timer_writel(val, reg) writel_relaxed((val), timer_base + (reg)) > + > +static void __iomem *timer_base; > + > static u64 notrace pxa_read_sched_clock(void) > { > - return readl_relaxed(OSCR); So here there is a change which is not explained in the changelog (timer_base offset). Even it is obvious for me because I am used to see this kind of code, that would deserve a better description in the changelog. > + return timer_readl(OSCR); > } > [ ... ] > +static void __init pxa_timer_dt_init(struct device_node *np) > +{ > + struct clk *clk; > + int irq; > + > + /* timer registers are shared with watchdog timer */ > + timer_base = of_iomap(np, 0); > + if (!timer_base) > + panic("%s: unable to map resource\n", np->name); > + > + clk = of_clk_get(np, 0); > + if (IS_ERR(clk)) > + panic("%s: unable to get clk\n", np->name); > + clk_prepare_enable(clk); > + > + /* we are only interested in OS-timer0 irq */ > + irq = irq_of_parse_and_map(np, 0); > + if (irq <= 0) > + panic("%s: unable to parse OS-timer0 irq\n", np->name); Is the 'panic' desirable ? The clksrc-of is written in a way to use different clocks, no ? > + > + pxa_timer_common_init(irq, clk_get_rate(clk)); > +} > +CLOCKSOURCE_OF_DECLARE(pxa_timer, "marvell,pxa-timer", pxa_timer_dt_init); > + > +/* > + * Legacy timer init for non device-tree boards. > + */ > +void __init pxa_timer_init(void) > +{ > + unsigned long clock_tick_rate = get_clock_tick_rate(); > + > + timer_base = io_p2v(0x40a00000); > + pxa_timer_common_init(IRQ_OST0, clock_tick_rate); > } > -- <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] clocksource: add device-tree support for PXA timer 2014-07-03 12:05 ` Daniel Lezcano @ 2014-07-03 17:31 ` Robert Jarzmik -1 siblings, 0 replies; 20+ messages in thread From: Robert Jarzmik @ 2014-07-03 17:31 UTC (permalink / raw) To: Daniel Lezcano Cc: Haojian Zhuang, Thomas Gleixner, linux-arm-kernel, linux-kernel Daniel Lezcano <daniel.lezcano@linaro.org> writes: >> -#include <mach/regs-ost.h> >> #include <mach/irqs.h> >> +#include <mach/hardware.h> > > Now as the driver is in 'drivers', do not reference the headers files in > mach. Moving the driver to the drivers directory implies some cleanup with the > headers dependencies. I don't see that very possible. Or said another way, I don't see how the irq number, IRQ_OST0 (in mach/irqs.h) can be guessed for non device-tree configuration. >> +#define OSMR0 0x00 /* */ >> +#define OSMR1 0x04 /* */ >> +#define OSMR2 0x08 /* */ >> +#define OSMR3 0x0C /* */ >> +#define OSMR4 0x80 /* */ > > Can you please remove those unused empty comment or fill them with something > appropriate. Sure. > >> +#define OSCR 0x10 /* OS Timer Counter Register */ >> +#define OSCR4 0x40 /* OS Timer Counter Register */ >> +#define OMCR4 0xC0 /* */ >> +#define OSSR 0x14 /* OS Timer Status Register */ >> +#define OWER 0x18 /* OS Timer Watchdog Enable Register */ >> +#define OIER 0x1C /* OS Timer Interrupt Enable Register */ >> + >> +#define OSSR_M3 (1 << 3) /* Match status channel 3 */ >> +#define OSSR_M2 (1 << 2) /* Match status channel 2 */ >> +#define OSSR_M1 (1 << 1) /* Match status channel 1 */ >> +#define OSSR_M0 (1 << 0) /* Match status channel 0 */ >> + >> +#define OWER_WME (1 << 0) /* Watchdog Match Enable */ >> + >> +#define OIER_E3 (1 << 3) /* Interrupt enable channel 3 */ >> +#define OIER_E2 (1 << 2) /* Interrupt enable channel 2 */ >> +#define OIER_E1 (1 << 1) /* Interrupt enable channel 1 */ >> +#define OIER_E0 (1 << 0) /* Interrupt enable channel 0 */ > > Is it possible to do some cleanups around regs-ost.h and here in order to remove > the unused macros. Also, it seems some define will be duplicate as they are > shared with the watchdog. Any plan to fix that ? For the cleanup, yes, will do. For the watchdog I don't have any plan yet. This patch's purpose is only to bring the PXA time source to drivers/clocksource, and make it compatible with both device-tree and non device-tree builds. >> @@ -33,9 +60,14 @@ >> * calls to sched_clock() which should always be the case in practice. >> */ >> >> +#define timer_readl(reg) readl_relaxed(timer_base + (reg)) >> +#define timer_writel(val, reg) writel_relaxed((val), timer_base + (reg)) >> + >> +static void __iomem *timer_base; >> + >> static u64 notrace pxa_read_sched_clock(void) >> { >> - return readl_relaxed(OSCR); > > So here there is a change which is not explained in the changelog (timer_base > offset). > > Even it is obvious for me because I am used to see this kind of code, that would > deserve a better description in the changelog. OK, I'll add the backward compatibility explanation with non device-tree builds, and the necessary timer_base iomem hard encoded value. And the Janus double face explanation of the driver (both DT and non-DT oriented). Another question brought up by this : if I remove all 'mach/' includes, I loose io_p2v() right ? How can I guess timer_base then ? >> + /* we are only interested in OS-timer0 irq */ >> + irq = irq_of_parse_and_map(np, 0); >> + if (irq <= 0) >> + panic("%s: unable to parse OS-timer0 irq\n", np->name); > > Is the 'panic' desirable ? The clksrc-of is written in a way to use different > clocks, no ? Good question. Maybe not, I followed the same rationale as in orion-timer, which is : - as this timer is the only possible timer for PXA boards, and because without it the kernel boot will stall (scheduling will be blocked), it's better to panic early that to remain stalled. Isn't this a good approach ? -- Robert ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/3] clocksource: add device-tree support for PXA timer @ 2014-07-03 17:31 ` Robert Jarzmik 0 siblings, 0 replies; 20+ messages in thread From: Robert Jarzmik @ 2014-07-03 17:31 UTC (permalink / raw) To: linux-arm-kernel Daniel Lezcano <daniel.lezcano@linaro.org> writes: >> -#include <mach/regs-ost.h> >> #include <mach/irqs.h> >> +#include <mach/hardware.h> > > Now as the driver is in 'drivers', do not reference the headers files in > mach. Moving the driver to the drivers directory implies some cleanup with the > headers dependencies. I don't see that very possible. Or said another way, I don't see how the irq number, IRQ_OST0 (in mach/irqs.h) can be guessed for non device-tree configuration. >> +#define OSMR0 0x00 /* */ >> +#define OSMR1 0x04 /* */ >> +#define OSMR2 0x08 /* */ >> +#define OSMR3 0x0C /* */ >> +#define OSMR4 0x80 /* */ > > Can you please remove those unused empty comment or fill them with something > appropriate. Sure. > >> +#define OSCR 0x10 /* OS Timer Counter Register */ >> +#define OSCR4 0x40 /* OS Timer Counter Register */ >> +#define OMCR4 0xC0 /* */ >> +#define OSSR 0x14 /* OS Timer Status Register */ >> +#define OWER 0x18 /* OS Timer Watchdog Enable Register */ >> +#define OIER 0x1C /* OS Timer Interrupt Enable Register */ >> + >> +#define OSSR_M3 (1 << 3) /* Match status channel 3 */ >> +#define OSSR_M2 (1 << 2) /* Match status channel 2 */ >> +#define OSSR_M1 (1 << 1) /* Match status channel 1 */ >> +#define OSSR_M0 (1 << 0) /* Match status channel 0 */ >> + >> +#define OWER_WME (1 << 0) /* Watchdog Match Enable */ >> + >> +#define OIER_E3 (1 << 3) /* Interrupt enable channel 3 */ >> +#define OIER_E2 (1 << 2) /* Interrupt enable channel 2 */ >> +#define OIER_E1 (1 << 1) /* Interrupt enable channel 1 */ >> +#define OIER_E0 (1 << 0) /* Interrupt enable channel 0 */ > > Is it possible to do some cleanups around regs-ost.h and here in order to remove > the unused macros. Also, it seems some define will be duplicate as they are > shared with the watchdog. Any plan to fix that ? For the cleanup, yes, will do. For the watchdog I don't have any plan yet. This patch's purpose is only to bring the PXA time source to drivers/clocksource, and make it compatible with both device-tree and non device-tree builds. >> @@ -33,9 +60,14 @@ >> * calls to sched_clock() which should always be the case in practice. >> */ >> >> +#define timer_readl(reg) readl_relaxed(timer_base + (reg)) >> +#define timer_writel(val, reg) writel_relaxed((val), timer_base + (reg)) >> + >> +static void __iomem *timer_base; >> + >> static u64 notrace pxa_read_sched_clock(void) >> { >> - return readl_relaxed(OSCR); > > So here there is a change which is not explained in the changelog (timer_base > offset). > > Even it is obvious for me because I am used to see this kind of code, that would > deserve a better description in the changelog. OK, I'll add the backward compatibility explanation with non device-tree builds, and the necessary timer_base iomem hard encoded value. And the Janus double face explanation of the driver (both DT and non-DT oriented). Another question brought up by this : if I remove all 'mach/' includes, I loose io_p2v() right ? How can I guess timer_base then ? >> + /* we are only interested in OS-timer0 irq */ >> + irq = irq_of_parse_and_map(np, 0); >> + if (irq <= 0) >> + panic("%s: unable to parse OS-timer0 irq\n", np->name); > > Is the 'panic' desirable ? The clksrc-of is written in a way to use different > clocks, no ? Good question. Maybe not, I followed the same rationale as in orion-timer, which is : - as this timer is the only possible timer for PXA boards, and because without it the kernel boot will stall (scheduling will be blocked), it's better to panic early that to remain stalled. Isn't this a good approach ? -- Robert ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] clocksource: add device-tree support for PXA timer 2014-07-03 17:31 ` Robert Jarzmik @ 2014-07-03 17:39 ` Robert Jarzmik -1 siblings, 0 replies; 20+ messages in thread From: Robert Jarzmik @ 2014-07-03 17:39 UTC (permalink / raw) To: Daniel Lezcano Cc: Haojian Zhuang, Thomas Gleixner, linux-arm-kernel, linux-kernel Robert Jarzmik <robert.jarzmik@free.fr> writes: > Daniel Lezcano <daniel.lezcano@linaro.org> writes: > >>> -#include <mach/regs-ost.h> >>> #include <mach/irqs.h> >>> +#include <mach/hardware.h> >> >> Now as the driver is in 'drivers', do not reference the headers files in >> mach. Moving the driver to the drivers directory implies some cleanup with the >> headers dependencies. > I don't see that very possible. > Or said another way, I don't see how the irq number, IRQ_OST0 (in mach/irqs.h) > can be guessed for non device-tree configuration. Oh yeah, a simple parameter to pxa_init_timer() will do the trick ... > Another question brought up by this : if I remove all 'mach/' includes, I loose > io_p2v() right ? How can I guess timer_base then ? And same answer here, a simple parameter to pxa_init_timer() will solve this too. Cheers. -- Robert ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/3] clocksource: add device-tree support for PXA timer @ 2014-07-03 17:39 ` Robert Jarzmik 0 siblings, 0 replies; 20+ messages in thread From: Robert Jarzmik @ 2014-07-03 17:39 UTC (permalink / raw) To: linux-arm-kernel Robert Jarzmik <robert.jarzmik@free.fr> writes: > Daniel Lezcano <daniel.lezcano@linaro.org> writes: > >>> -#include <mach/regs-ost.h> >>> #include <mach/irqs.h> >>> +#include <mach/hardware.h> >> >> Now as the driver is in 'drivers', do not reference the headers files in >> mach. Moving the driver to the drivers directory implies some cleanup with the >> headers dependencies. > I don't see that very possible. > Or said another way, I don't see how the irq number, IRQ_OST0 (in mach/irqs.h) > can be guessed for non device-tree configuration. Oh yeah, a simple parameter to pxa_init_timer() will do the trick ... > Another question brought up by this : if I remove all 'mach/' includes, I loose > io_p2v() right ? How can I guess timer_base then ? And same answer here, a simple parameter to pxa_init_timer() will solve this too. Cheers. -- Robert ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] clocksource: add device-tree support for PXA timer 2014-07-03 17:31 ` Robert Jarzmik @ 2014-07-04 6:18 ` Daniel Lezcano -1 siblings, 0 replies; 20+ messages in thread From: Daniel Lezcano @ 2014-07-04 6:18 UTC (permalink / raw) To: Robert Jarzmik Cc: Haojian Zhuang, Thomas Gleixner, linux-arm-kernel, linux-kernel On 07/03/2014 07:31 PM, Robert Jarzmik wrote: > Daniel Lezcano <daniel.lezcano@linaro.org> writes: > >>> -#include <mach/regs-ost.h> >>> #include <mach/irqs.h> >>> +#include <mach/hardware.h> >> >> Now as the driver is in 'drivers', do not reference the headers files in >> mach. Moving the driver to the drivers directory implies some cleanup with the >> headers dependencies. > I don't see that very possible. > Or said another way, I don't see how the irq number, IRQ_OST0 (in mach/irqs.h) > can be guessed for non device-tree configuration. > >>> +#define OSMR0 0x00 /* */ >>> +#define OSMR1 0x04 /* */ >>> +#define OSMR2 0x08 /* */ >>> +#define OSMR3 0x0C /* */ >>> +#define OSMR4 0x80 /* */ >> >> Can you please remove those unused empty comment or fill them with something >> appropriate. > Sure. > >> >>> +#define OSCR 0x10 /* OS Timer Counter Register */ >>> +#define OSCR4 0x40 /* OS Timer Counter Register */ >>> +#define OMCR4 0xC0 /* */ >>> +#define OSSR 0x14 /* OS Timer Status Register */ >>> +#define OWER 0x18 /* OS Timer Watchdog Enable Register */ >>> +#define OIER 0x1C /* OS Timer Interrupt Enable Register */ >>> + >>> +#define OSSR_M3 (1 << 3) /* Match status channel 3 */ >>> +#define OSSR_M2 (1 << 2) /* Match status channel 2 */ >>> +#define OSSR_M1 (1 << 1) /* Match status channel 1 */ >>> +#define OSSR_M0 (1 << 0) /* Match status channel 0 */ >>> + >>> +#define OWER_WME (1 << 0) /* Watchdog Match Enable */ >>> + >>> +#define OIER_E3 (1 << 3) /* Interrupt enable channel 3 */ >>> +#define OIER_E2 (1 << 2) /* Interrupt enable channel 2 */ >>> +#define OIER_E1 (1 << 1) /* Interrupt enable channel 1 */ >>> +#define OIER_E0 (1 << 0) /* Interrupt enable channel 0 */ >> >> Is it possible to do some cleanups around regs-ost.h and here in order to remove >> the unused macros. Also, it seems some define will be duplicate as they are >> shared with the watchdog. Any plan to fix that ? > For the cleanup, yes, will do. > > For the watchdog I don't have any plan yet. This patch's purpose is only to > bring the PXA time source to drivers/clocksource, and make it compatible with > both device-tree and non device-tree builds. > >>> @@ -33,9 +60,14 @@ >>> * calls to sched_clock() which should always be the case in practice. >>> */ >>> >>> +#define timer_readl(reg) readl_relaxed(timer_base + (reg)) >>> +#define timer_writel(val, reg) writel_relaxed((val), timer_base + (reg)) >>> + >>> +static void __iomem *timer_base; >>> + >>> static u64 notrace pxa_read_sched_clock(void) >>> { >>> - return readl_relaxed(OSCR); >> >> So here there is a change which is not explained in the changelog (timer_base >> offset). >> >> Even it is obvious for me because I am used to see this kind of code, that would >> deserve a better description in the changelog. > OK, I'll add the backward compatibility explanation with non device-tree builds, > and the necessary timer_base iomem hard encoded value. And the Janus double face > explanation of the driver (both DT and non-DT oriented). > > Another question brought up by this : if I remove all 'mach/' includes, I loose > io_p2v() right ? How can I guess timer_base then ? > >>> + /* we are only interested in OS-timer0 irq */ >>> + irq = irq_of_parse_and_map(np, 0); >>> + if (irq <= 0) >>> + panic("%s: unable to parse OS-timer0 irq\n", np->name); >> >> Is the 'panic' desirable ? The clksrc-of is written in a way to use different >> clocks, no ? > Good question. > > Maybe not, I followed the same rationale as in orion-timer, which is : > - as this timer is the only possible timer for PXA boards, and because without > it the kernel boot will stall (scheduling will be blocked), it's better to > panic early that to remain stalled. There isn't the arm global timer ? > Isn't this a good approach ? I suppose we can live with that. IMO, the right fix would be in clksrc-of to pr_crit a message when an initialization fails. But that means to change all the init functions for all drivers which is out of the scope of this patchset. -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/3] clocksource: add device-tree support for PXA timer @ 2014-07-04 6:18 ` Daniel Lezcano 0 siblings, 0 replies; 20+ messages in thread From: Daniel Lezcano @ 2014-07-04 6:18 UTC (permalink / raw) To: linux-arm-kernel On 07/03/2014 07:31 PM, Robert Jarzmik wrote: > Daniel Lezcano <daniel.lezcano@linaro.org> writes: > >>> -#include <mach/regs-ost.h> >>> #include <mach/irqs.h> >>> +#include <mach/hardware.h> >> >> Now as the driver is in 'drivers', do not reference the headers files in >> mach. Moving the driver to the drivers directory implies some cleanup with the >> headers dependencies. > I don't see that very possible. > Or said another way, I don't see how the irq number, IRQ_OST0 (in mach/irqs.h) > can be guessed for non device-tree configuration. > >>> +#define OSMR0 0x00 /* */ >>> +#define OSMR1 0x04 /* */ >>> +#define OSMR2 0x08 /* */ >>> +#define OSMR3 0x0C /* */ >>> +#define OSMR4 0x80 /* */ >> >> Can you please remove those unused empty comment or fill them with something >> appropriate. > Sure. > >> >>> +#define OSCR 0x10 /* OS Timer Counter Register */ >>> +#define OSCR4 0x40 /* OS Timer Counter Register */ >>> +#define OMCR4 0xC0 /* */ >>> +#define OSSR 0x14 /* OS Timer Status Register */ >>> +#define OWER 0x18 /* OS Timer Watchdog Enable Register */ >>> +#define OIER 0x1C /* OS Timer Interrupt Enable Register */ >>> + >>> +#define OSSR_M3 (1 << 3) /* Match status channel 3 */ >>> +#define OSSR_M2 (1 << 2) /* Match status channel 2 */ >>> +#define OSSR_M1 (1 << 1) /* Match status channel 1 */ >>> +#define OSSR_M0 (1 << 0) /* Match status channel 0 */ >>> + >>> +#define OWER_WME (1 << 0) /* Watchdog Match Enable */ >>> + >>> +#define OIER_E3 (1 << 3) /* Interrupt enable channel 3 */ >>> +#define OIER_E2 (1 << 2) /* Interrupt enable channel 2 */ >>> +#define OIER_E1 (1 << 1) /* Interrupt enable channel 1 */ >>> +#define OIER_E0 (1 << 0) /* Interrupt enable channel 0 */ >> >> Is it possible to do some cleanups around regs-ost.h and here in order to remove >> the unused macros. Also, it seems some define will be duplicate as they are >> shared with the watchdog. Any plan to fix that ? > For the cleanup, yes, will do. > > For the watchdog I don't have any plan yet. This patch's purpose is only to > bring the PXA time source to drivers/clocksource, and make it compatible with > both device-tree and non device-tree builds. > >>> @@ -33,9 +60,14 @@ >>> * calls to sched_clock() which should always be the case in practice. >>> */ >>> >>> +#define timer_readl(reg) readl_relaxed(timer_base + (reg)) >>> +#define timer_writel(val, reg) writel_relaxed((val), timer_base + (reg)) >>> + >>> +static void __iomem *timer_base; >>> + >>> static u64 notrace pxa_read_sched_clock(void) >>> { >>> - return readl_relaxed(OSCR); >> >> So here there is a change which is not explained in the changelog (timer_base >> offset). >> >> Even it is obvious for me because I am used to see this kind of code, that would >> deserve a better description in the changelog. > OK, I'll add the backward compatibility explanation with non device-tree builds, > and the necessary timer_base iomem hard encoded value. And the Janus double face > explanation of the driver (both DT and non-DT oriented). > > Another question brought up by this : if I remove all 'mach/' includes, I loose > io_p2v() right ? How can I guess timer_base then ? > >>> + /* we are only interested in OS-timer0 irq */ >>> + irq = irq_of_parse_and_map(np, 0); >>> + if (irq <= 0) >>> + panic("%s: unable to parse OS-timer0 irq\n", np->name); >> >> Is the 'panic' desirable ? The clksrc-of is written in a way to use different >> clocks, no ? > Good question. > > Maybe not, I followed the same rationale as in orion-timer, which is : > - as this timer is the only possible timer for PXA boards, and because without > it the kernel boot will stall (scheduling will be blocked), it's better to > panic early that to remain stalled. There isn't the arm global timer ? > Isn't this a good approach ? I suppose we can live with that. IMO, the right fix would be in clksrc-of to pr_crit a message when an initialization fails. But that means to change all the init functions for all drivers which is out of the scope of this patchset. -- <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] clocksource: add device-tree support for PXA timer 2014-07-04 6:18 ` Daniel Lezcano @ 2014-07-04 19:46 ` Robert Jarzmik -1 siblings, 0 replies; 20+ messages in thread From: Robert Jarzmik @ 2014-07-04 19:46 UTC (permalink / raw) To: Daniel Lezcano Cc: Haojian Zhuang, Thomas Gleixner, linux-arm-kernel, linux-kernel Daniel Lezcano <daniel.lezcano@linaro.org> writes: >> Good question. >> >> Maybe not, I followed the same rationale as in orion-timer, which is : >> - as this timer is the only possible timer for PXA boards, and because without >> it the kernel boot will stall (scheduling will be blocked), it's better to >> panic early that to remain stalled. > > There isn't the arm global timer ? Nope, it's for Cortex-A9 AFAIK, and my poor platform is an old ARMv5 SoC. > >> Isn't this a good approach ? > > I suppose we can live with that. IMO, the right fix would be in clksrc-of to > pr_crit a message when an initialization fails. But that means to change all the > init functions for all drivers which is out of the scope of this patchset. OK, you convinced me. I'll trade the panic() for a pr_crit(). Good idea, maybe it will trigger a quest of a white knight for the other drivers :) Cheers. -- Robert ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/3] clocksource: add device-tree support for PXA timer @ 2014-07-04 19:46 ` Robert Jarzmik 0 siblings, 0 replies; 20+ messages in thread From: Robert Jarzmik @ 2014-07-04 19:46 UTC (permalink / raw) To: linux-arm-kernel Daniel Lezcano <daniel.lezcano@linaro.org> writes: >> Good question. >> >> Maybe not, I followed the same rationale as in orion-timer, which is : >> - as this timer is the only possible timer for PXA boards, and because without >> it the kernel boot will stall (scheduling will be blocked), it's better to >> panic early that to remain stalled. > > There isn't the arm global timer ? Nope, it's for Cortex-A9 AFAIK, and my poor platform is an old ARMv5 SoC. > >> Isn't this a good approach ? > > I suppose we can live with that. IMO, the right fix would be in clksrc-of to > pr_crit a message when an initialization fails. But that means to change all the > init functions for all drivers which is out of the scope of this patchset. OK, you convinced me. I'll trade the panic() for a pr_crit(). Good idea, maybe it will trigger a quest of a white knight for the other drivers :) Cheers. -- Robert ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/3] ARM: add CLKSRC_OF dependency for PXA 2014-06-21 22:09 ` Robert Jarzmik @ 2014-06-21 22:09 ` Robert Jarzmik -1 siblings, 0 replies; 20+ messages in thread From: Robert Jarzmik @ 2014-06-21 22:09 UTC (permalink / raw) To: Haojian Zhuang, Daniel Lezcano, Thomas Gleixner Cc: linux-arm-kernel, linux-kernel, Robert Jarzmik, Russell King Select CLKSRC_OF for PXA architectures, as the clocksource driver can be also initialized from device-tree. Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> Cc: Russell King <linux@arm.linux.org.uk> --- arch/arm/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index db3c541..c300b68 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -646,6 +646,7 @@ config ARCH_PXA select AUTO_ZRELADDR select CLKDEV_LOOKUP select CLKSRC_MMIO + select CLKSRC_OF if OF select GENERIC_CLOCKEVENTS select GPIO_PXA select HAVE_IDE -- 2.0.0.rc2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] ARM: add CLKSRC_OF dependency for PXA @ 2014-06-21 22:09 ` Robert Jarzmik 0 siblings, 0 replies; 20+ messages in thread From: Robert Jarzmik @ 2014-06-21 22:09 UTC (permalink / raw) To: linux-arm-kernel Select CLKSRC_OF for PXA architectures, as the clocksource driver can be also initialized from device-tree. Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> Cc: Russell King <linux@arm.linux.org.uk> --- arch/arm/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index db3c541..c300b68 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -646,6 +646,7 @@ config ARCH_PXA select AUTO_ZRELADDR select CLKDEV_LOOKUP select CLKSRC_MMIO + select CLKSRC_OF if OF select GENERIC_CLOCKEVENTS select GPIO_PXA select HAVE_IDE -- 2.0.0.rc2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] clocksource: move PXA timer to clocksource framework 2014-06-21 22:09 ` Robert Jarzmik @ 2014-06-29 14:15 ` Robert Jarzmik -1 siblings, 0 replies; 20+ messages in thread From: Robert Jarzmik @ 2014-06-29 14:15 UTC (permalink / raw) To: Haojian Zhuang, Daniel Lezcano, Thomas Gleixner Cc: linux-arm-kernel, linux-kernel Robert Jarzmik <robert.jarzmik@free.fr> writes: > Move time.c from arch/arm/mach-pxa/time.c to > drivers/clocksource/pxa_timer.c. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> Ping ? Cheers. -- Robert ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] clocksource: move PXA timer to clocksource framework @ 2014-06-29 14:15 ` Robert Jarzmik 0 siblings, 0 replies; 20+ messages in thread From: Robert Jarzmik @ 2014-06-29 14:15 UTC (permalink / raw) To: linux-arm-kernel Robert Jarzmik <robert.jarzmik@free.fr> writes: > Move time.c from arch/arm/mach-pxa/time.c to > drivers/clocksource/pxa_timer.c. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> Ping ? Cheers. -- Robert ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] clocksource: move PXA timer to clocksource framework 2014-06-21 22:09 ` Robert Jarzmik @ 2014-07-03 4:37 ` Haojian Zhuang -1 siblings, 0 replies; 20+ messages in thread From: Haojian Zhuang @ 2014-07-03 4:37 UTC (permalink / raw) To: Robert Jarzmik Cc: Daniel Lezcano, Thomas Gleixner, linux-arm-kernel, linux-kernel On Sun, Jun 22, 2014 at 6:09 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote: > Move time.c from arch/arm/mach-pxa/time.c to > drivers/clocksource/pxa_timer.c. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > --- > arch/arm/mach-pxa/Makefile | 2 +- > drivers/clocksource/Makefile | 1 + > arch/arm/mach-pxa/time.c => drivers/clocksource/pxa_timer.c | 0 > 3 files changed, 2 insertions(+), 1 deletion(-) > rename arch/arm/mach-pxa/time.c => drivers/clocksource/pxa_timer.c (100%) > > diff --git a/arch/arm/mach-pxa/Makefile b/arch/arm/mach-pxa/Makefile > index 648867a..2fe1824 100644 > --- a/arch/arm/mach-pxa/Makefile > +++ b/arch/arm/mach-pxa/Makefile > @@ -4,7 +4,7 @@ > > # Common support (must be linked before board specific support) > obj-y += clock.o devices.o generic.o irq.o \ > - time.o reset.o > + reset.o > obj-$(CONFIG_PM) += pm.o sleep.o standby.o > > # Generic drivers that other drivers may depend upon > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile > index 98cb6c5..e224125 100644 > --- a/drivers/clocksource/Makefile > +++ b/drivers/clocksource/Makefile > @@ -19,6 +19,7 @@ obj-$(CONFIG_ARCH_BCM2835) += bcm2835_timer.o > obj-$(CONFIG_ARCH_MARCO) += timer-marco.o > obj-$(CONFIG_ARCH_MOXART) += moxart_timer.o > obj-$(CONFIG_ARCH_MXS) += mxs_timer.o > +obj-$(CONFIG_ARCH_PXA) += pxa_timer.o > obj-$(CONFIG_ARCH_PRIMA2) += timer-prima2.o > obj-$(CONFIG_ARCH_U300) += timer-u300.o > obj-$(CONFIG_SUN4I_TIMER) += sun4i_timer.o > diff --git a/arch/arm/mach-pxa/time.c b/drivers/clocksource/pxa_timer.c > similarity index 100% > rename from arch/arm/mach-pxa/time.c > rename to drivers/clocksource/pxa_timer.c > -- > 2.0.0.rc2 > Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com> Acked for all these three patches. Regards Haojian ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] clocksource: move PXA timer to clocksource framework @ 2014-07-03 4:37 ` Haojian Zhuang 0 siblings, 0 replies; 20+ messages in thread From: Haojian Zhuang @ 2014-07-03 4:37 UTC (permalink / raw) To: linux-arm-kernel On Sun, Jun 22, 2014 at 6:09 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote: > Move time.c from arch/arm/mach-pxa/time.c to > drivers/clocksource/pxa_timer.c. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > --- > arch/arm/mach-pxa/Makefile | 2 +- > drivers/clocksource/Makefile | 1 + > arch/arm/mach-pxa/time.c => drivers/clocksource/pxa_timer.c | 0 > 3 files changed, 2 insertions(+), 1 deletion(-) > rename arch/arm/mach-pxa/time.c => drivers/clocksource/pxa_timer.c (100%) > > diff --git a/arch/arm/mach-pxa/Makefile b/arch/arm/mach-pxa/Makefile > index 648867a..2fe1824 100644 > --- a/arch/arm/mach-pxa/Makefile > +++ b/arch/arm/mach-pxa/Makefile > @@ -4,7 +4,7 @@ > > # Common support (must be linked before board specific support) > obj-y += clock.o devices.o generic.o irq.o \ > - time.o reset.o > + reset.o > obj-$(CONFIG_PM) += pm.o sleep.o standby.o > > # Generic drivers that other drivers may depend upon > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile > index 98cb6c5..e224125 100644 > --- a/drivers/clocksource/Makefile > +++ b/drivers/clocksource/Makefile > @@ -19,6 +19,7 @@ obj-$(CONFIG_ARCH_BCM2835) += bcm2835_timer.o > obj-$(CONFIG_ARCH_MARCO) += timer-marco.o > obj-$(CONFIG_ARCH_MOXART) += moxart_timer.o > obj-$(CONFIG_ARCH_MXS) += mxs_timer.o > +obj-$(CONFIG_ARCH_PXA) += pxa_timer.o > obj-$(CONFIG_ARCH_PRIMA2) += timer-prima2.o > obj-$(CONFIG_ARCH_U300) += timer-u300.o > obj-$(CONFIG_SUN4I_TIMER) += sun4i_timer.o > diff --git a/arch/arm/mach-pxa/time.c b/drivers/clocksource/pxa_timer.c > similarity index 100% > rename from arch/arm/mach-pxa/time.c > rename to drivers/clocksource/pxa_timer.c > -- > 2.0.0.rc2 > Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com> Acked for all these three patches. Regards Haojian ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2014-07-04 19:46 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-21 22:09 [PATCH 1/3] clocksource: move PXA timer to clocksource framework Robert Jarzmik 2014-06-21 22:09 ` Robert Jarzmik 2014-06-21 22:09 ` [PATCH 2/3] clocksource: add device-tree support for PXA timer Robert Jarzmik 2014-06-21 22:09 ` Robert Jarzmik 2014-07-03 12:05 ` Daniel Lezcano 2014-07-03 12:05 ` Daniel Lezcano 2014-07-03 17:31 ` Robert Jarzmik 2014-07-03 17:31 ` Robert Jarzmik 2014-07-03 17:39 ` Robert Jarzmik 2014-07-03 17:39 ` Robert Jarzmik 2014-07-04 6:18 ` Daniel Lezcano 2014-07-04 6:18 ` Daniel Lezcano 2014-07-04 19:46 ` Robert Jarzmik 2014-07-04 19:46 ` Robert Jarzmik 2014-06-21 22:09 ` [PATCH 3/3] ARM: add CLKSRC_OF dependency for PXA Robert Jarzmik 2014-06-21 22:09 ` Robert Jarzmik 2014-06-29 14:15 ` [PATCH 1/3] clocksource: move PXA timer to clocksource framework Robert Jarzmik 2014-06-29 14:15 ` Robert Jarzmik 2014-07-03 4:37 ` Haojian Zhuang 2014-07-03 4:37 ` Haojian Zhuang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.