Re: [PATCH 07/30 v2] clocksource/drivers/ixp4xx: Add driver

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 21/02/2019 16:44, Linus Walleij wrote:
> This adds a new slightly rewritten timer driver for the
> Intel IXP4xx clocksource, clockevent and delay timer.
> 
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Clocksource/timer maintainers: I am requesting an ACK for
> this once you're happy with the driver, as I intend to
> merge all of this IXP4xx rework through ARM SoC.
> ---

Hi Linus,

a couple of questions:

 - why __raw_write|read ?

 - can you consider using the timer_of API ?

>  MAINTAINERS                                |   2 +
>  drivers/clocksource/Kconfig                |   7 +
>  drivers/clocksource/Makefile               |   1 +
>  drivers/clocksource/timer-ixp4xx.c         | 249 +++++++++++++++++++++
>  include/linux/platform_data/timer-ixp4xx.h |  11 +
>  5 files changed, 270 insertions(+)
>  create mode 100644 drivers/clocksource/timer-ixp4xx.c
>  create mode 100644 include/linux/platform_data/timer-ixp4xx.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6c161bd82238..a2fb67b75026 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1651,9 +1651,11 @@ M:	Krzysztof Halasa <khalasa@piap.pl>
>  L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>  S:	Maintained
>  F:	arch/arm/mach-ixp4xx/
> +F:	drivers/clocksource/timer-ixp4xx.c
>  F:	drivers/gpio/gpio-ixp4xx.c
>  F:	drivers/irqchip/irq-ixp4xx.c
>  F:	include/linux/irqchip/irq-ixp4xx.h
> +F:	include/linux/platform_data/timer-ixp4xx.h
>  
>  ARM/INTEL RESEARCH IMOTE/STARGATE 2 MACHINE SUPPORT
>  M:	Jonathan Cameron <jic23@cam.ac.uk>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index a9e26f6a81a1..d290d6c6576d 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -69,6 +69,13 @@ config FTTMR010_TIMER
>  	  Enables support for the Faraday Technology timer block
>  	  FTTMR010.
>  
> +config IXP4XX_TIMER
> +	bool "Intel XScale IXP4xx timer driver" if COMPILE_TEST
> +	depends on HAS_IOMEM
> +	select CLKSRC_MMIO
> +	help
> +	  Enables support for the Intel XScale IXP4xx SoC timer.
> +
>  config ROCKCHIP_TIMER
>  	bool "Rockchip timer driver" if COMPILE_TEST
>  	depends on ARM || ARM64
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index cdd210ff89ea..18158257ebb3 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_OMAP_DM_TIMER)	+= timer-ti-dm.o
>  obj-$(CONFIG_DW_APB_TIMER)	+= dw_apb_timer.o
>  obj-$(CONFIG_DW_APB_TIMER_OF)	+= dw_apb_timer_of.o
>  obj-$(CONFIG_FTTMR010_TIMER)	+= timer-fttmr010.o
> +obj-$(CONFIG_IXP4XX_TIMER)	+= timer-ixp4xx.o
>  obj-$(CONFIG_ROCKCHIP_TIMER)      += timer-rockchip.o
>  obj-$(CONFIG_CLKSRC_NOMADIK_MTU)	+= nomadik-mtu.o
>  obj-$(CONFIG_CLKSRC_DBX500_PRCMU)	+= clksrc-dbx500-prcmu.o
> diff --git a/drivers/clocksource/timer-ixp4xx.c b/drivers/clocksource/timer-ixp4xx.c
> new file mode 100644
> index 000000000000..fa78f80792db
> --- /dev/null
> +++ b/drivers/clocksource/timer-ixp4xx.c
> @@ -0,0 +1,249 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * IXP4 timer driver
> + * Copyright (C) 2019 Linus Walleij <linus.walleij@linaro.org>
> + *
> + * Based on arch/arm/mach-ixp4xx/common.c
> + * Copyright 2002 (C) Intel Corporation
> + * Copyright 2003-2004 (C) MontaVista, Software, Inc.
> + * Copyright (C) Deepak Saxena <dsaxena@plexity.net>
> + */
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/clockchips.h>
> +#include <linux/clocksource.h>
> +#include <linux/sched_clock.h>
> +#include <linux/slab.h>
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +/* Goes away with OF conversion */
> +#include <linux/platform_data/timer-ixp4xx.h>
> +
> +/*
> + * Constants to make it easy to access Timer Control/Status registers
> + */
> +#define IXP4XX_OSTS_OFFSET	0x00  /* Continuous Timestamp */
> +#define IXP4XX_OST1_OFFSET	0x04  /* Timer 1 Timestamp */
> +#define IXP4XX_OSRT1_OFFSET	0x08  /* Timer 1 Reload */
> +#define IXP4XX_OST2_OFFSET	0x0C  /* Timer 2 Timestamp */
> +#define IXP4XX_OSRT2_OFFSET	0x10  /* Timer 2 Reload */
> +#define IXP4XX_OSWT_OFFSET	0x14  /* Watchdog Timer */
> +#define IXP4XX_OSWE_OFFSET	0x18  /* Watchdog Enable */
> +#define IXP4XX_OSWK_OFFSET	0x1C  /* Watchdog Key */
> +#define IXP4XX_OSST_OFFSET	0x20  /* Timer Status */
> +
> +/*
> + * Timer register values and bit definitions
> + */
> +#define IXP4XX_OST_ENABLE		0x00000001
> +#define IXP4XX_OST_ONE_SHOT		0x00000002
> +/* Low order bits of reload value ignored */
> +#define IXP4XX_OST_RELOAD_MASK		0x00000003
> +#define IXP4XX_OST_DISABLED		0x00000000
> +#define IXP4XX_OSST_TIMER_1_PEND	0x00000001
> +#define IXP4XX_OSST_TIMER_2_PEND	0x00000002
> +#define IXP4XX_OSST_TIMER_TS_PEND	0x00000004
> +#define IXP4XX_OSST_TIMER_WDOG_PEND	0x00000008
> +#define IXP4XX_OSST_TIMER_WARM_RESET	0x00000010
> +
> +#define	IXP4XX_WDT_KEY			0x0000482E
> +#define	IXP4XX_WDT_RESET_ENABLE		0x00000001
> +#define	IXP4XX_WDT_IRQ_ENABLE		0x00000002
> +#define	IXP4XX_WDT_COUNT_ENABLE		0x00000004
> +
> +struct ixp4xx_timer {
> +	void __iomem *base;
> +	unsigned int tick_rate;
> +	u32 latch;
> +	struct clock_event_device clkevt;
> +#ifdef CONFIG_ARM
> +	struct delay_timer delay_timer;
> +#endif
> +};
> +
> +/*
> + * A local singleton used by sched_clock and delay timer reads, which are
> + * fast and stateless
> + */
> +static struct ixp4xx_timer *local_ixp4xx_timer;
> +
> +static inline struct ixp4xx_timer *
> +to_ixp4xx_timer(struct clock_event_device *evt)
> +{
> +	return container_of(evt, struct ixp4xx_timer, clkevt);
> +}
> +
> +static u64 notrace ixp4xx_read_sched_clock(void)
> +{
> +	return __raw_readl(local_ixp4xx_timer->base + IXP4XX_OSTS_OFFSET);
> +}
> +
> +static u64 ixp4xx_clocksource_read(struct clocksource *c)
> +{
> +	return __raw_readl(local_ixp4xx_timer->base + IXP4XX_OSTS_OFFSET);
> +}
> +
> +static irqreturn_t ixp4xx_timer_interrupt(int irq, void *dev_id)
> +{
> +	struct ixp4xx_timer *tmr = dev_id;
> +	struct clock_event_device *evt = &tmr->clkevt;
> +
> +	/* Clear Pending Interrupt */
> +	__raw_writel(IXP4XX_OSST_TIMER_1_PEND,
> +		     tmr->base + IXP4XX_OSST_OFFSET);
> +
> +	evt->event_handler(evt);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int ixp4xx_set_next_event(unsigned long cycles,
> +				 struct clock_event_device *evt)
> +{
> +	struct ixp4xx_timer *tmr = to_ixp4xx_timer(evt);
> +	u32 val;
> +
> +	val = __raw_readl(tmr->base + IXP4XX_OSRT1_OFFSET);
> +	/* Keep enable/oneshot bits */
> +	val &= IXP4XX_OST_RELOAD_MASK;
> +	__raw_writel((cycles & ~IXP4XX_OST_RELOAD_MASK) | val,
> +		     tmr->base + IXP4XX_OSRT1_OFFSET);
> +
> +	return 0;
> +}
> +
> +static int ixp4xx_shutdown(struct clock_event_device *evt)
> +{
> +	struct ixp4xx_timer *tmr = to_ixp4xx_timer(evt);
> +	u32 val;
> +
> +	val = __raw_readl(tmr->base + IXP4XX_OSRT1_OFFSET);
> +	val &= ~IXP4XX_OST_ENABLE;
> +	__raw_writel(val, tmr->base + IXP4XX_OSRT1_OFFSET);
> +
> +	return 0;
> +}
> +
> +static int ixp4xx_set_oneshot(struct clock_event_device *evt)
> +{
> +	struct ixp4xx_timer *tmr = to_ixp4xx_timer(evt);
> +
> +	__raw_writel(IXP4XX_OST_ENABLE | IXP4XX_OST_ONE_SHOT,
> +		     tmr->base + IXP4XX_OSRT1_OFFSET);
> +
> +	return 0;
> +}
> +
> +static int ixp4xx_set_periodic(struct clock_event_device *evt)
> +{
> +	struct ixp4xx_timer *tmr = to_ixp4xx_timer(evt);
> +	u32 val;
> +
> +	val = tmr->latch & ~IXP4XX_OST_RELOAD_MASK;
> +	val |= IXP4XX_OST_ENABLE;
> +	__raw_writel(val, tmr->base + IXP4XX_OSRT1_OFFSET);
> +
> +	return 0;
> +}
> +
> +static int ixp4xx_resume(struct clock_event_device *evt)
> +{
> +	struct ixp4xx_timer *tmr = to_ixp4xx_timer(evt);
> +	u32 val;
> +
> +	val = __raw_readl(tmr->base + IXP4XX_OSRT1_OFFSET);
> +	val |= IXP4XX_OST_ENABLE;
> +	__raw_writel(val, tmr->base + IXP4XX_OSRT1_OFFSET);
> +
> +	return 0;
> +}
> +
> +/*
> + * IXP4xx timer tick
> + * We use OS timer1 on the CPU for the timer tick and the timestamp
> + * counter as a source of real clock ticks to account for missed jiffies.
> + */
> +static __init int ixp4xx_timer_register(void __iomem *base,
> +					int timer_irq,
> +					unsigned int timer_freq)
> +{
> +	struct ixp4xx_timer *tmr;
> +	int ret;
> +
> +	tmr = kzalloc(sizeof(*tmr), GFP_KERNEL);
> +	if (!tmr)
> +		return -ENOMEM;
> +	tmr->base = base;
> +	tmr->tick_rate = timer_freq;
> +
> +	/*
> +	 * The timer register doesn't allow to specify the two least
> +	 * significant bits of the timeout value and assumes them being zero.
> +	 * So make sure the latch is the best value with the two least
> +	 * significant bits unset.
> +	 */
> +	tmr->latch = DIV_ROUND_CLOSEST(timer_freq,
> +				       (IXP4XX_OST_RELOAD_MASK + 1) * HZ)
> +		* (IXP4XX_OST_RELOAD_MASK + 1);
> +
> +	local_ixp4xx_timer = tmr;
> +
> +	/* Reset/disable counter */
> +	__raw_writel(0, tmr->base + IXP4XX_OSRT1_OFFSET);
> +
> +	/* Clear any pending interrupt on timer 1 */
> +	__raw_writel(IXP4XX_OSST_TIMER_1_PEND,
> +		     tmr->base + IXP4XX_OSST_OFFSET);
> +
> +	/* Reset time-stamp counter */
> +	__raw_writel(0, tmr->base + IXP4XX_OSTS_OFFSET);
> +
> +	clocksource_mmio_init(NULL, "OSTS", timer_freq, 200, 32,
> +			      ixp4xx_clocksource_read);
> +
> +	tmr->clkevt.name = "ixp4xx timer1";
> +	tmr->clkevt.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
> +	tmr->clkevt.rating = 200;
> +	tmr->clkevt.set_state_shutdown = ixp4xx_shutdown;
> +	tmr->clkevt.set_state_periodic = ixp4xx_set_periodic;
> +	tmr->clkevt.set_state_oneshot = ixp4xx_set_oneshot;
> +	tmr->clkevt.tick_resume = ixp4xx_resume;
> +	tmr->clkevt.set_next_event = ixp4xx_set_next_event;
> +	tmr->clkevt.cpumask = cpumask_of(0);
> +	tmr->clkevt.irq = timer_irq;
> +	ret = request_irq(timer_irq, ixp4xx_timer_interrupt,
> +			  IRQF_TIMER, "IXP4XX-TIMER1", tmr);
> +	if (ret) {
> +		pr_crit("no timer IRQ\n");
> +		return -ENODEV;
> +	}
> +	clockevents_config_and_register(&tmr->clkevt, timer_freq,
> +					0xf, 0xfffffffe);
> +
> +#ifdef CONFIG_ARM
> +	sched_clock_register(ixp4xx_read_sched_clock, 32, timer_freq);
> +#endif
> +
> +	return 0;
> +}
> +
> +/**
> + * ixp4xx_timer_setup() - Timer setup function to be called from boardfiles
> + * @timerbase: physical base of timer block
> + * @timer_irq: Linux IRQ number for the timer
> + * @timer_freq: Fixed frequency of the timer
> + */
> +void __init ixp4xx_timer_setup(resource_size_t timerbase,
> +			       int timer_irq,
> +			       unsigned int timer_freq)
> +{
> +	void __iomem *base;
> +
> +	base = ioremap(timerbase, 0x100);
> +	if (!base) {
> +		pr_crit("IXP4xx: can't remap timer\n");
> +		return;
> +	}
> +	ixp4xx_timer_register(base, timer_irq, timer_freq);
> +}
> +EXPORT_SYMBOL_GPL(ixp4xx_timer_setup);
> diff --git a/include/linux/platform_data/timer-ixp4xx.h b/include/linux/platform_data/timer-ixp4xx.h
> new file mode 100644
> index 000000000000..ee92ae7edaed
> --- /dev/null
> +++ b/include/linux/platform_data/timer-ixp4xx.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __TIMER_IXP4XX_H
> +#define __TIMER_IXP4XX_H
> +
> +#include <linux/ioport.h>
> +
> +void __init ixp4xx_timer_setup(resource_size_t timerbase,
> +			       int timer_irq,
> +			       unsigned int timer_freq);
> +
> +#endif
> 


-- 
 <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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel