From: rmallon@gmail.com (Ryan Mallon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: at91: make st soc independent
Date: Fri, 09 Dec 2011 09:46:10 +1100 [thread overview]
Message-ID: <4EE13E32.7030905@gmail.com> (raw)
In-Reply-To: <1323358532-21135-1-git-send-email-plagnioj@jcrosoft.com>
On 09/12/11 02:35, Jean-Christophe PLAGNIOL-VILLARD wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Nicolas Ferre <nicolas.ferre@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);
> }
>
> /* ......................................................................... */
next prev parent reply other threads:[~2011-12-08 22:46 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-08 15:35 [PATCH] ARM: at91: make st soc independent Jean-Christophe PLAGNIOL-VILLARD
2011-12-08 22:46 ` Ryan Mallon [this message]
2011-12-09 6:16 ` Jean-Christophe PLAGNIOL-VILLARD
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4EE13E32.7030905@gmail.com \
--to=rmallon@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.