[PATCH] ARM: at91: make st soc independent
Ryan Mallon
rmallon at gmail.com
Thu Dec 8 17:46:10 EST 2011
On 09/12/11 02:35, Jean-Christophe PLAGNIOL-VILLARD wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj at jcrosoft.com>
> Cc: Nicolas Ferre <nicolas.ferre at atmel.com>
> ---
> arch/arm/mach-at91/at91rm9200.c | 4 +-
> arch/arm/mach-at91/at91rm9200_time.c | 37 ++++++++++++++++----------
> arch/arm/mach-at91/generic.h | 1 +
> arch/arm/mach-at91/include/mach/at91_st.h | 32 +++++++++++++++-------
> arch/arm/mach-at91/include/mach/at91rm9200.h | 2 +-
> drivers/watchdog/at91rm9200_wdt.c | 8 +++---
> 6 files changed, 53 insertions(+), 31 deletions(-)
Some comments below,
~Ryan
> diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c
> index 9163d7d..6b819c0 100644
> --- a/arch/arm/mach-at91/at91rm9200.c
> +++ b/arch/arm/mach-at91/at91rm9200.c
> @@ -294,8 +294,8 @@ static void at91rm9200_reset(void)
> /*
> * Perform a hardware reset with the use of the Watchdog timer.
> */
> - at91_sys_write(AT91_ST_WDMR, AT91_ST_RSTEN | AT91_ST_EXTEN | 1);
> - at91_sys_write(AT91_ST_CR, AT91_ST_WDRST);
> + at91_st_write(AT91_ST_WDMR, AT91_ST_RSTEN | AT91_ST_EXTEN | 1);
> + at91_st_write(AT91_ST_CR, AT91_ST_WDRST);
> }
>
> /* --------------------------------------------------------------------
> diff --git a/arch/arm/mach-at91/at91rm9200_time.c b/arch/arm/mach-at91/at91rm9200_time.c
> index a028cdf..52d4b45 100644
> --- a/arch/arm/mach-at91/at91rm9200_time.c
> +++ b/arch/arm/mach-at91/at91rm9200_time.c
> @@ -43,9 +43,9 @@ static inline unsigned long read_CRTR(void)
> {
> unsigned long x1, x2;
>
> - x1 = at91_sys_read(AT91_ST_CRTR);
> + x1 = at91_st_read(AT91_ST_CRTR);
> do {
> - x2 = at91_sys_read(AT91_ST_CRTR);
> + x2 = at91_st_read(AT91_ST_CRTR);
> if (x1 == x2)
> break;
> x1 = x2;
> @@ -58,7 +58,7 @@ static inline unsigned long read_CRTR(void)
> */
> static irqreturn_t at91rm9200_timer_interrupt(int irq, void *dev_id)
> {
> - u32 sr = at91_sys_read(AT91_ST_SR) & irqmask;
> + u32 sr = at91_st_read(AT91_ST_SR) & irqmask;
>
> /*
> * irqs should be disabled here, but as the irq is shared they are only
> @@ -110,22 +110,22 @@ static void
> clkevt32k_mode(enum clock_event_mode mode, struct clock_event_device *dev)
> {
> /* Disable and flush pending timer interrupts */
> - at91_sys_write(AT91_ST_IDR, AT91_ST_PITS | AT91_ST_ALMS);
> - (void) at91_sys_read(AT91_ST_SR);
> + at91_st_write(AT91_ST_IDR, AT91_ST_PITS | AT91_ST_ALMS);
> + (void) at91_st_read(AT91_ST_SR);
Drop the void cast or check the return value.
>
> last_crtr = read_CRTR();
> switch (mode) {
> case CLOCK_EVT_MODE_PERIODIC:
> /* PIT for periodic irqs; fixed rate of 1/HZ */
> irqmask = AT91_ST_PITS;
> - at91_sys_write(AT91_ST_PIMR, RM9200_TIMER_LATCH);
> + at91_st_write(AT91_ST_PIMR, RM9200_TIMER_LATCH);
> break;
> case CLOCK_EVT_MODE_ONESHOT:
> /* ALM for oneshot irqs, set by next_event()
> * before 32 seconds have passed
> */
> irqmask = AT91_ST_ALMS;
> - at91_sys_write(AT91_ST_RTAR, last_crtr);
> + at91_st_write(AT91_ST_RTAR, last_crtr);
> break;
> case CLOCK_EVT_MODE_SHUTDOWN:
> case CLOCK_EVT_MODE_UNUSED:
> @@ -133,7 +133,7 @@ clkevt32k_mode(enum clock_event_mode mode, struct clock_event_device *dev)
> irqmask = 0;
> break;
> }
> - at91_sys_write(AT91_ST_IER, irqmask);
> + at91_st_write(AT91_ST_IER, irqmask);
> }
>
> static int
> @@ -156,12 +156,12 @@ clkevt32k_next_event(unsigned long delta, struct clock_event_device *dev)
> alm = read_CRTR();
>
> /* Cancel any pending alarm; flush any pending IRQ */
> - at91_sys_write(AT91_ST_RTAR, alm);
> - (void) at91_sys_read(AT91_ST_SR);
> + at91_st_write(AT91_ST_RTAR, alm);
> + (void) at91_st_read(AT91_ST_SR);
Same here, remove the void cast or check the return value.
>
> /* Schedule alarm by writing RTAR. */
> alm += delta;
> - at91_sys_write(AT91_ST_RTAR, alm);
> + at91_st_write(AT91_ST_RTAR, alm);
>
> return status;
> }
> @@ -175,15 +175,24 @@ static struct clock_event_device clkevt = {
> .set_mode = clkevt32k_mode,
> };
>
> +void __iomem *at91_st_base;
> +
> +void __init at91rm9200_ioremap_st(u32 addr)
> +{
> + at91_st_base = ioremap(AT91RM9200_BASE_ST, 256);
> + if (!at91_st_base)
> + panic("Impossible to ioremap ST\n");
> +}
I'm unsure about this. Only at91rm9200 has the ST, so is it worth
ioremapping it and having a global variable? At the very least could we
make at91_st_base static and move at91_st_read/write here.
> +
> /*
> * ST (system timer) module supports both clockevents and clocksource.
> */
> void __init at91rm9200_timer_init(void)
> {
> /* Disable all timer interrupts, and clear any pending ones */
> - at91_sys_write(AT91_ST_IDR,
> + at91_st_write(AT91_ST_IDR,
> AT91_ST_PITS | AT91_ST_WDOVF | AT91_ST_RTTINC | AT91_ST_ALMS);
> - (void) at91_sys_read(AT91_ST_SR);
> + (void) at91_st_read(AT91_ST_SR);
Drop the cast.
>
> /* Make IRQs happen for the system timer */
> setup_irq(AT91_ID_SYS, &at91rm9200_timer_irq);
> @@ -192,7 +201,7 @@ void __init at91rm9200_timer_init(void)
> * directly for the clocksource and all clockevents, after adjusting
> * its prescaler from the 1 Hz default.
> */
> - at91_sys_write(AT91_ST_RTMR, 1);
> + at91_st_write(AT91_ST_RTMR, 1);
>
> /* Setup timer clockevent, with minimum of two ticks (important!!) */
> clkevt.mult = div_sc(AT91_SLOW_CLOCK, NSEC_PER_SEC, clkevt.shift);
> diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
> index 4030958..445c00d 100644
> --- a/arch/arm/mach-at91/generic.h
> +++ b/arch/arm/mach-at91/generic.h
> @@ -28,6 +28,7 @@ extern void __init at91_aic_init(unsigned int priority[]);
>
> /* Timer */
> struct sys_timer;
> +extern void at91rm9200_ioremap_st(u32 addr);
> extern struct sys_timer at91rm9200_timer;
> extern void at91sam926x_ioremap_pit(u32 addr);
> extern struct sys_timer at91sam926x_timer;
> diff --git a/arch/arm/mach-at91/include/mach/at91_st.h b/arch/arm/mach-at91/include/mach/at91_st.h
> index 8847173..969aac2 100644
> --- a/arch/arm/mach-at91/include/mach/at91_st.h
> +++ b/arch/arm/mach-at91/include/mach/at91_st.h
> @@ -16,34 +16,46 @@
> #ifndef AT91_ST_H
> #define AT91_ST_H
>
> -#define AT91_ST_CR (AT91_ST + 0x00) /* Control Register */
> +#ifndef __ASSEMBLY__
> +extern void __iomem *at91_st_base;
> +
> +#define at91_st_read(field) \
> + __raw_readl(at91_st_base + field)
> +
> +#define at91_st_write(field, value) \
> + __raw_writel(value, at91_st_base + field);
Static inline functions are nicer for readability/type checking.
> +#else
> +.extern at91_st_base
Typo: remove the . in front of the extern. Has this been compile tested?
> +#endif
> +
> +#define AT91_ST_CR 0x00 /* Control Register */
> #define AT91_ST_WDRST (1 << 0) /* Watchdog Timer Restart */
>
> -#define AT91_ST_PIMR (AT91_ST + 0x04) /* Period Interval Mode Register */
> +#define AT91_ST_PIMR 0x04 /* Period Interval Mode Register */
> #define AT91_ST_PIV (0xffff << 0) /* Period Interval Value */
>
> -#define AT91_ST_WDMR (AT91_ST + 0x08) /* Watchdog Mode Register */
> +#define AT91_ST_WDMR 0x08 /* Watchdog Mode Register */
> #define AT91_ST_WDV (0xffff << 0) /* Watchdog Counter Value */
> #define AT91_ST_RSTEN (1 << 16) /* Reset Enable */
> #define AT91_ST_EXTEN (1 << 17) /* External Signal Assertion Enable */
>
> -#define AT91_ST_RTMR (AT91_ST + 0x0c) /* Real-time Mode Register */
> +#define AT91_ST_RTMR 0x0c /* Real-time Mode Register */
> #define AT91_ST_RTPRES (0xffff << 0) /* Real-time Prescalar Value */
>
> -#define AT91_ST_SR (AT91_ST + 0x10) /* Status Register */
> +#define AT91_ST_SR 0x10 /* Status Register */
> #define AT91_ST_PITS (1 << 0) /* Period Interval Timer Status */
> #define AT91_ST_WDOVF (1 << 1) /* Watchdog Overflow */
> #define AT91_ST_RTTINC (1 << 2) /* Real-time Timer Increment */
> #define AT91_ST_ALMS (1 << 3) /* Alarm Status */
>
> -#define AT91_ST_IER (AT91_ST + 0x14) /* Interrupt Enable Register */
> -#define AT91_ST_IDR (AT91_ST + 0x18) /* Interrupt Disable Register */
> -#define AT91_ST_IMR (AT91_ST + 0x1c) /* Interrupt Mask Register */
> +#define AT91_ST_IER 0x14 /* Interrupt Enable Register */
> +#define AT91_ST_IDR 0x18 /* Interrupt Disable Register */
> +#define AT91_ST_IMR 0x1c /* Interrupt Mask Register */
>
> -#define AT91_ST_RTAR (AT91_ST + 0x20) /* Real-time Alarm Register */
> +#define AT91_ST_RTAR 0x20 /* Real-time Alarm Register */
> #define AT91_ST_ALMV (0xfffff << 0) /* Alarm Value */
>
> -#define AT91_ST_CRTR (AT91_ST + 0x24) /* Current Real-time Register */
> +#define AT91_ST_CRTR 0x24 /* Current Real-time Register */
> #define AT91_ST_CRTV (0xfffff << 0) /* Current Real-Time Value */
>
> #endif
> diff --git a/arch/arm/mach-at91/include/mach/at91rm9200.h b/arch/arm/mach-at91/include/mach/at91rm9200.h
> index bacb511..8583009 100644
> --- a/arch/arm/mach-at91/include/mach/at91rm9200.h
> +++ b/arch/arm/mach-at91/include/mach/at91rm9200.h
> @@ -80,7 +80,6 @@
> * System Peripherals (offset from AT91_BASE_SYS)
> */
> #define AT91_PMC (0xfffffc00 - AT91_BASE_SYS) /* Power Management Controller */
> -#define AT91_ST (0xfffffd00 - AT91_BASE_SYS) /* System Timer */
> #define AT91_MC (0xffffff00 - AT91_BASE_SYS) /* Memory Controllers */
>
> #define AT91RM9200_BASE_DBGU AT91_BASE_DBGU0 /* Debug Unit */
> @@ -88,6 +87,7 @@
> #define AT91RM9200_BASE_PIOB 0xfffff600 /* PIO Controller B */
> #define AT91RM9200_BASE_PIOC 0xfffff800 /* PIO Controller C */
> #define AT91RM9200_BASE_PIOD 0xfffffa00 /* PIO Controller D */
> +#define AT91RM9200_BASE_ST 0xfffffd00 /* System Timer */
> #define AT91RM9200_BASE_RTC 0xfffffe00 /* Real-Time Clock */
>
> #define AT91_USART0 AT91RM9200_BASE_US0
> diff --git a/drivers/watchdog/at91rm9200_wdt.c b/drivers/watchdog/at91rm9200_wdt.c
> index b3046dc..7ceefd2 100644
> --- a/drivers/watchdog/at91rm9200_wdt.c
> +++ b/drivers/watchdog/at91rm9200_wdt.c
> @@ -51,7 +51,7 @@ static unsigned long at91wdt_busy;
> */
> static inline void at91_wdt_stop(void)
> {
> - at91_sys_write(AT91_ST_WDMR, AT91_ST_EXTEN);
> + at91_st_write(AT91_ST_WDMR, AT91_ST_EXTEN);
> }
>
> /*
> @@ -59,9 +59,9 @@ static inline void at91_wdt_stop(void)
> */
> static inline void at91_wdt_start(void)
> {
> - at91_sys_write(AT91_ST_WDMR, AT91_ST_EXTEN | AT91_ST_RSTEN |
> + at91_st_write(AT91_ST_WDMR, AT91_ST_EXTEN | AT91_ST_RSTEN |
> (((65536 * wdt_time) >> 8) & AT91_ST_WDV));
> - at91_sys_write(AT91_ST_CR, AT91_ST_WDRST);
> + at91_st_write(AT91_ST_CR, AT91_ST_WDRST);
> }
>
> /*
> @@ -69,7 +69,7 @@ static inline void at91_wdt_start(void)
> */
> static inline void at91_wdt_reload(void)
> {
> - at91_sys_write(AT91_ST_CR, AT91_ST_WDRST);
> + at91_st_write(AT91_ST_CR, AT91_ST_WDRST);
> }
>
> /* ......................................................................... */
More information about the linux-arm-kernel
mailing list