Skip to contentSkip to conversation

ARM: at91: make st soc independent

PATCH New Jean-Christophe PLAGNIOL-VILLARD 2 participants 3 messages Dec 8, 2011 – Dec 9, 2011

Subsystems (4)
Arm (32-bit) Arm sub-architectures Arm/Microchip (AT91) SoC support Watchdog device drivers
lore
Share & tools
lore

Jean-Christophe PLAGNIOL-VILLARD

author
#0 lore raw

PATCH 1/1 ARM: at91: make st soc independent

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Nicolas Ferre <[email protected]>
---
 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(-)
arch/arm/mach-at91/at91rm9200.c +2 -2
index 9163d7d..6b819c0 100644
@@ -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);
 }
 
 /* --------------------------------------------------------------------
arch/arm/mach-at91/at91rm9200_time.c +23 -14
index a028cdf..52d4b45 100644
@@ -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);
 
 	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);
 
 	/* 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");
+}
+
 /*
  * 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);
 
 	/* 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);
arch/arm/mach-at91/generic.h +1 -0
index 4030958..445c00d 100644
@@ -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;
arch/arm/mach-at91/include/mach/at91_st.h +22 -10
index 8847173..969aac2 100644
@@ -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);
+#else
+.extern at91_st_base
+#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
arch/arm/mach-at91/include/mach/at91rm9200.h +1 -1
index bacb511..8583009 100644
@@ -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
drivers/watchdog/at91rm9200_wdt.c +4 -4
index b3046dc..7ceefd2 100644
@@ -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);
 }
 
 /* ......................................................................... */
Signature · 1 line
1.7.7
#1 lore raw

PATCH 1/1 ARM: at91: make st soc independent

Quoted 10 lines · Jean-Christophe PLAGNIOL-VILLARD
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Nicolas Ferre <[email protected]>
---
 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

Quoted 41 lines
arch/arm/mach-at91/at91rm9200.c +2 -2
index 9163d7d..6b819c0 100644
@@ -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);
 }
 
 /* --------------------------------------------------------------------
arch/arm/mach-at91/at91rm9200_time.c +3 -3
index a028cdf..52d4b45 100644
@@ -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

arch/arm/mach-at91/at91rm9200_time.c static void · line 114

Quoted 7 lines
arch/arm/mach-at91/at91rm9200_time.c +2 -2
@@ -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.

Quoted 29 lines
arch/arm/mach-at91/at91rm9200_time.c +3 -3
 	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

arch/arm/mach-at91/at91rm9200_time.c clkevt32k_next_event(unsigned long delta, struct clock_event_device *dev) · line 160

Quoted 7 lines
arch/arm/mach-at91/at91rm9200_time.c +2 -2
@@ -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.

Quoted 10 lines
arch/arm/mach-at91/at91rm9200_time.c +1 -1
 	/* 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,

arch/arm/mach-at91/at91rm9200_time.c static struct clock_event_device clkevt = { · line 185

Quoted 10 lines
arch/arm/mach-at91/at91rm9200_time.c +8 -0
 };
 
+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.

arch/arm/mach-at91/at91rm9200_time.c static struct clock_event_device clkevt = { · line 195

Quoted 12 lines
arch/arm/mach-at91/at91rm9200_time.c +3 -2
+
 /*
  * 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.

Quoted 31 lines
arch/arm/mach-at91/at91rm9200_time.c +1 -1
 	/* 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);
arch/arm/mach-at91/generic.h +1 -0
index 4030958..445c00d 100644
@@ -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;
arch/arm/mach-at91/include/mach/at91_st.h +0 -0
index 8847173..969aac2 100644
@@ -16,34 +16,46 @@
 #ifndef AT91_ST_H
 #define AT91_ST_H

arch/arm/mach-at91/include/mach/at91_st.h line 26

Quoted 10 lines
arch/arm/mach-at91/include/mach/at91_st.h +8 -1
 
-#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.

arch/arm/mach-at91/include/mach/at91_st.h line 28

Quoted 2 lines
arch/arm/mach-at91/include/mach/at91_st.h +2 -0
+#else
+.extern at91_st_base

Typo: remove the . in front of the extern. Has this been compile tested?

Quoted 96 lines
arch/arm/mach-at91/include/mach/at91_st.h +12 -9
+#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
arch/arm/mach-at91/include/mach/at91rm9200.h +1 -1
index bacb511..8583009 100644
@@ -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
drivers/watchdog/at91rm9200_wdt.c +4 -4
index b3046dc..7ceefd2 100644
@@ -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);
 }
 
 /* ......................................................................... */
author
#2 lore raw

PATCH 1/1 ARM: at91: make st soc independent

Quoted 55 lines · Ryan Mallon
Quoted 10 lines · Jean-Christophe PLAGNIOL-VILLARD
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Nicolas Ferre <[email protected]>
---
 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

Quoted 38 lines
arch/arm/mach-at91/at91rm9200.c +2 -2
index 9163d7d..6b819c0 100644
@@ -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);
 }
 
 /* --------------------------------------------------------------------
arch/arm/mach-at91/at91rm9200_time.c +3 -3
index a028cdf..52d4b45 100644
@@ -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;
Quoted 13 lines

arch/arm/mach-at91/at91rm9200_time.c static void · line 114

Quoted 10 lines
arch/arm/mach-at91/at91rm9200_time.c +2 -2
 	/*
 	 * 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.

this is not he scope of this patch

the change are by script

Quoted 32 lines
Quoted 32 lines
arch/arm/mach-at91/at91rm9200_time.c +3 -3
 	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();
Quoted 8 lines

arch/arm/mach-at91/at91rm9200_time.c clkevt32k_next_event(unsigned long delta, struct clock_event_device *dev) · line 160

Quoted 5 lines
arch/arm/mach-at91/at91rm9200_time.c +2 -2
 	/* 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.

ditti

Quoted 13 lines
Quoted 13 lines
arch/arm/mach-at91/at91rm9200_time.c +1 -1
 	/* 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,
 };
Quoted 13 lines

arch/arm/mach-at91/at91rm9200_time.c static struct clock_event_device clkevt = { · line 185

Quoted 8 lines
arch/arm/mach-at91/at91rm9200_time.c +8 -0
+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.

no we need it at different place in the kernel

Quoted 49 lines

arch/arm/mach-at91/at91rm9200_time.c static struct clock_event_device clkevt = { · line 195

Quoted 13 lines
arch/arm/mach-at91/at91rm9200_time.c +3 -2
+
 /*
  * 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.

Quoted 32 lines
arch/arm/mach-at91/at91rm9200_time.c +1 -1
 	/* 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);
arch/arm/mach-at91/generic.h +1 -0
index 4030958..445c00d 100644
@@ -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;
arch/arm/mach-at91/include/mach/at91_st.h +0 -0
index 8847173..969aac2 100644
@@ -16,34 +16,46 @@
 #ifndef AT91_ST_H
 #define AT91_ST_H
Quoted 12 lines

arch/arm/mach-at91/include/mach/at91_st.h line 26

Quoted 9 lines
arch/arm/mach-at91/include/mach/at91_st.h +8 -1
-#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.

sorry no this will improve nothing

Quoted 6 lines

arch/arm/mach-at91/include/mach/at91_st.h line 28

Quoted 3 lines
arch/arm/mach-at91/include/mach/at91_st.h +2 -0
+#else
+.extern at91_st_base

Typo: remove the . in front of the extern. Has this been compile tested?

certainly not it's assembly
.extern is right

Best Regards,
J.