ARM: ixp4xx: Add "ask" handler for timer interrupts

PATCH New Alexander Shiyan 4 participants 17 messages Dec 1, 2012 – Dec 3, 2012

Subsystems (7)

Network drivers Arm (32-bit) Arm sub-architectures Input drivers +3 more subsystems

lore

Share & tools

lore

Alexander Shiyan

author

2012-12-01 07:33 UTC #0 lore raw

PATCH 1/1 ARM: ixp4xx: Add "ask" handler for timer interrupts

Rather than modify the EOI flags directly in the timer interrupt, let's deal with these flags in the "ask" procedure.

Signed-off-by Alexander Shiyan <[email protected]>

---
 arch/arm/mach-ixp4xx/common.c      |   34 +++++++++++++++++++++++++---------
 drivers/input/misc/ixp4xx-beeper.c |    3 ---
 2 files changed, 25 insertions(+), 12 deletions(-)

arch/arm/mach-ixp4xx/common.c +25 -9

index 8c0c0e2..27046ec 100644
@@ -202,10 +202,28 @@ static void ixp4xx_irq_mask(struct irq_data *d)
 
 static void ixp4xx_irq_ack(struct irq_data *d)
 {
-	int line = (d->irq < 32) ? irq2gpio[d->irq] : -1;
+	int line;
 
-	if (line >= 0)
-		*IXP4XX_GPIO_GPISR = (1 << line);
+	switch (d->irq) {
+	case IRQ_IXP4XX_TIMER1:
+		*IXP4XX_OSST = IXP4XX_OSST_TIMER_1_PEND;
+		break;
+	case IRQ_IXP4XX_TIMER2:
+		*IXP4XX_OSST = IXP4XX_OSST_TIMER_2_PEND;
+		break;
+	case IRQ_IXP4XX_TIMESTAMP:
+		*IXP4XX_OSST = IXP4XX_OSST_TIMER_TS_PEND;
+		break;
+	case IRQ_IXP4XX_WDOG:
+		*IXP4XX_OSST = IXP4XX_OSST_TIMER_WDOG_PEND;
+		break;
+	default:
+		line = (d->irq < 32) ? irq2gpio[d->irq] : -1;
+
+		if (line >= 0)
+			*IXP4XX_GPIO_GPISR = (1 << line);
+		break;
+	}
 }
 
 /*

@@ -247,6 +265,10 @@ void __init ixp4xx_init_irq(void)
 	/* Disable all interrupt */
 	*IXP4XX_ICMR = 0x0; 
 
+	/* Clear Timer Pending Interrupts */
+	*IXP4XX_OSST = IXP4XX_OSST_TIMER_1_PEND | IXP4XX_OSST_TIMER_2_PEND |
+		       IXP4XX_OSST_TIMER_TS_PEND | IXP4XX_OSST_TIMER_WDOG_PEND;
+
 	if (cpu_is_ixp46x() || cpu_is_ixp43x()) {
 		/* Route upper 32 sources to IRQ instead of FIQ */
 		*IXP4XX_ICLR2 = 0x00;

@@ -274,9 +296,6 @@ static irqreturn_t ixp4xx_timer_interrupt(int irq, void *dev_id)
 {
 	struct clock_event_device *evt = dev_id;
 
-	/* Clear Pending Interrupt by writing '1' to it */
-	*IXP4XX_OSST = IXP4XX_OSST_TIMER_1_PEND;
-
 	evt->event_handler(evt);
 
 	return IRQ_HANDLED;

@@ -294,9 +313,6 @@ void __init ixp4xx_timer_init(void)
 	/* Reset/disable counter */
 	*IXP4XX_OSRT1 = 0;
 
-	/* Clear Pending Interrupt by writing '1' to it */
-	*IXP4XX_OSST = IXP4XX_OSST_TIMER_1_PEND;
-
 	/* Reset time-stamp counter */
 	*IXP4XX_OSTS = 0;

drivers/input/misc/ixp4xx-beeper.c +0 -3

index 6ab3dec..2923e47 100644
@@ -78,9 +78,6 @@ static int ixp4xx_spkr_event(struct input_dev *dev, unsigned int type, unsigned
 
 static irqreturn_t ixp4xx_spkr_interrupt(int irq, void *dev_id)
 {
-	/* clear interrupt */
-	*IXP4XX_OSST = IXP4XX_OSST_TIMER_2_PEND;
-
 	/* flip the beeper output */
 	*IXP4XX_GPIO_GPOUTR ^= (1 << (unsigned int) dev_id);

Signature · 1 line

1.7.8.6

Jason Cooper

2012-12-01 13:05 UTC #1 lore raw

PATCH 1/1 ARM: ixp4xx: Add "ask" handler for timer interrupts

Hi Alexander,

Quoted 2 lines · Alexander Shiyan

Rather than modify the EOI flags directly in the timer interrupt, let's deal with these flags in the "ask" procedure.

Please specify the procedure name instead of "ask", eg ixp4xx_irq_ack(). Same goes for the Subject line.

What brought about this change, is this part of a larger series?

thx,

Jason.

Quoted 96 lines

Signed-off-by Alexander Shiyan <[email protected]>

---
 arch/arm/mach-ixp4xx/common.c      |   34 +++++++++++++++++++++++++---------
 drivers/input/misc/ixp4xx-beeper.c |    3 ---
 2 files changed, 25 insertions(+), 12 deletions(-)

arch/arm/mach-ixp4xx/common.c +25 -9

index 8c0c0e2..27046ec 100644
@@ -202,10 +202,28 @@ static void ixp4xx_irq_mask(struct irq_data *d)
 
 static void ixp4xx_irq_ack(struct irq_data *d)
 {
-	int line = (d->irq < 32) ? irq2gpio[d->irq] : -1;
+	int line;
 
-	if (line >= 0)
-		*IXP4XX_GPIO_GPISR = (1 << line);
+	switch (d->irq) {
+	case IRQ_IXP4XX_TIMER1:
+		*IXP4XX_OSST = IXP4XX_OSST_TIMER_1_PEND;
+		break;
+	case IRQ_IXP4XX_TIMER2:
+		*IXP4XX_OSST = IXP4XX_OSST_TIMER_2_PEND;
+		break;
+	case IRQ_IXP4XX_TIMESTAMP:
+		*IXP4XX_OSST = IXP4XX_OSST_TIMER_TS_PEND;
+		break;
+	case IRQ_IXP4XX_WDOG:
+		*IXP4XX_OSST = IXP4XX_OSST_TIMER_WDOG_PEND;
+		break;
+	default:
+		line = (d->irq < 32) ? irq2gpio[d->irq] : -1;
+
+		if (line >= 0)
+			*IXP4XX_GPIO_GPISR = (1 << line);
+		break;
+	}
 }
 
 /*

@@ -247,6 +265,10 @@ void __init ixp4xx_init_irq(void)
 	/* Disable all interrupt */
 	*IXP4XX_ICMR = 0x0; 
 
+	/* Clear Timer Pending Interrupts */
+	*IXP4XX_OSST = IXP4XX_OSST_TIMER_1_PEND | IXP4XX_OSST_TIMER_2_PEND |
+		       IXP4XX_OSST_TIMER_TS_PEND | IXP4XX_OSST_TIMER_WDOG_PEND;
+
 	if (cpu_is_ixp46x() || cpu_is_ixp43x()) {
 		/* Route upper 32 sources to IRQ instead of FIQ */
 		*IXP4XX_ICLR2 = 0x00;

@@ -274,9 +296,6 @@ static irqreturn_t ixp4xx_timer_interrupt(int irq, void *dev_id)
 {
 	struct clock_event_device *evt = dev_id;
 
-	/* Clear Pending Interrupt by writing '1' to it */
-	*IXP4XX_OSST = IXP4XX_OSST_TIMER_1_PEND;
-
 	evt->event_handler(evt);
 
 	return IRQ_HANDLED;

@@ -294,9 +313,6 @@ void __init ixp4xx_timer_init(void)
 	/* Reset/disable counter */
 	*IXP4XX_OSRT1 = 0;
 
-	/* Clear Pending Interrupt by writing '1' to it */
-	*IXP4XX_OSST = IXP4XX_OSST_TIMER_1_PEND;
-
 	/* Reset time-stamp counter */
 	*IXP4XX_OSTS = 0;

drivers/input/misc/ixp4xx-beeper.c +0 -3

index 6ab3dec..2923e47 100644
@@ -78,9 +78,6 @@ static int ixp4xx_spkr_event(struct input_dev *dev, unsigned int type, unsigned
 
 static irqreturn_t ixp4xx_spkr_interrupt(int irq, void *dev_id)
 {
-	/* clear interrupt */
-	*IXP4XX_OSST = IXP4XX_OSST_TIMER_2_PEND;
-
 	/* flip the beeper output */
 	*IXP4XX_GPIO_GPOUTR ^= (1 << (unsigned int) dev_id);

Signature · 7 lines

1.7.8.6


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

Alexander Shiyan

author

2012-12-01 13:14 UTC #2 lore raw

Quoted 8 lines · Alexander Shiyan

Quoted 2 lines
Rather than modify the EOI flags directly in the timer interrupt, let's deal with these flags in the "ask" procedure.
Please specify the procedure name instead of "ask", eg ixp4xx_irq_ack(). Same goes for the Subject line.
What brought about this change, is this part of a larger series?

It all started with an attempt to clear the unneeded calls from drivers. Series not yet, just an initial attempt to put everything in its place.

---

Jason Cooper

2012-12-01 15:41 UTC #3 lore raw

PATCH 1/1 ARM: ixp4xx: Add "ask" handler for timer interrupts

Quoted 11 lines · Alexander Shiyan

Quoted 8 lines · Alexander Shiyan
Quoted 2 lines
Rather than modify the EOI flags directly in the timer interrupt, let's deal with these flags in the "ask" procedure.
Please specify the procedure name instead of "ask", eg ixp4xx_irq_ack(). Same goes for the Subject line.
What brought about this change, is this part of a larger series?
It all started with an attempt to clear the unneeded calls from drivers. Series not yet, just an initial attempt to put everything in its place.

Ok, sounds good. There's definitely a lot of cleanup work in ixp4xx. Drivers needing moved to drivers/, etc.

thx,

Jason.

Arnd Bergmann

2012-12-01 21:25 UTC #4 lore raw

PATCH 1/1 ARM: ixp4xx: Add "ask" handler for timer interrupts

In reply to Alexander Shiyan

arch/arm/mach-ixp4xx/common.c static void ixp4xx_irq_mask(struct irq_data *d) · line 219

Quoted 13 lines · Alexander Shiyan

arch/arm/mach-ixp4xx/common.c +13 -0

+       switch (d->irq) {
+       case IRQ_IXP4XX_TIMER1:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_1_PEND;
+               break;
+       case IRQ_IXP4XX_TIMER2:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_2_PEND;
+               break;
+       case IRQ_IXP4XX_TIMESTAMP:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_TS_PEND;
+               break;
+       case IRQ_IXP4XX_WDOG:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_WDOG_PEND;
+               break;

Since you are touching these lines, it probably makes sense to convert them to use writel_relaxed() in the process. Dereferencing a volatile pointer in order to do MMIO is strongly discouraged, see Documentation/volatile-considered-harmful.txt

Arnd

Jason Cooper

2012-12-01 21:45 UTC #5 lore raw

PATCH 1/1 ARM: ixp4xx: Add "ask" handler for timer interrupts

Quoted 15 lines · Arnd Bergmann

arch/arm/mach-ixp4xx/common.c static void ixp4xx_irq_mask(struct irq_data *d) · line 219

Quoted 13 lines · Alexander Shiyan

arch/arm/mach-ixp4xx/common.c +13 -0

+       switch (d->irq) {
+       case IRQ_IXP4XX_TIMER1:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_1_PEND;
+               break;
+       case IRQ_IXP4XX_TIMER2:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_2_PEND;
+               break;
+       case IRQ_IXP4XX_TIMESTAMP:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_TS_PEND;
+               break;
+       case IRQ_IXP4XX_WDOG:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_WDOG_PEND;
+               break;

Quoted 4 lines

Since you are touching these lines, it probably makes sense to convert them to use writel_relaxed() in the process. Dereferencing a volatile pointer in order to do MMIO is strongly discouraged, see Documentation/volatile-considered-harmful.txt

Arnd,

I took a quick look at the ixp4xx code when I saw this. It appears the entire sub-arch is written this way :-( Perhaps it would be better to do a cleanup patch before this one? I didn't mention it in my initial comment because it looks like quite a bit of work.

In either case, it's all cleanup, so it shouldn't cause a dependency headache.

Alexander, if you're so inclined, a cleanup series would be much appreciated. If you don't have the time, no problem, just make the changes suggested by Arnd and I and we'll get to the cleanup eventually.

thx,

Jason.

Arnd Bergmann

2012-12-02 00:11 UTC #6 lore raw

PATCH 1/1 ARM: ixp4xx: Add "ask" handler for timer interrupts

Quoted 28 lines · Jason Cooper

Quoted 15 lines · Arnd Bergmann
Review
Quoted 13 lines · Alexander Shiyan
(quoted diff) +13 -0
+       switch (d->irq) {
+       case IRQ_IXP4XX_TIMER1:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_1_PEND;
+               break;
+       case IRQ_IXP4XX_TIMER2:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_2_PEND;
+               break;
+       case IRQ_IXP4XX_TIMESTAMP:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_TS_PEND;
+               break;
+       case IRQ_IXP4XX_WDOG:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_WDOG_PEND;
+               break;
Quoted 4 lines
Since you are touching these lines, it probably makes sense to convert them to use writel_relaxed() in the process. Dereferencing a volatile pointer in order to do MMIO is strongly discouraged, see Documentation/volatile-considered-harmful.txt
Arnd,
I took a quick look at the ixp4xx code when I saw this. It appears the entire sub-arch is written this way :-( Perhaps it would be better to do a cleanup patch before this one? I didn't mention it in my initial comment because it looks like quite a bit of work.

Quoted 6 lines

In either case, it's all cleanup, so it shouldn't cause a dependency headache.
Alexander, if you're so inclined, a cleanup series would be much appreciated. If you don't have the time, no problem, just make the changes suggested by Arnd and I and we'll get to the cleanup eventually.

I got curious to how hard this would be and ended up with a patch.

Arnd

8<--------
[PATCH] ARM: ixp4xx: use proper __iomem annotations consistently

The ixp4xx platform on ARM is one of the remaining locations still using direct pointer dereferences for MMIO access. This patch should convert all known instances to use readl_relaxed/writel_relaxed.

I could not find a nice solution for mach/hardware.h, which is included by mach/io.h indirectly but actually requires MMIO accesses as defined in asm/io.h. Using a macro as a workaround helps, but the better solution in the long run would be to have a proper gpiolib driver rather than using a private API to do GPIO.

I did not touch the definitions used in drivers/usb/gadget/pxa25x_udc.c, they are shared with the pxa platform and ever more screwed up than the other ones in ixp4xx that I fixed.

In the process of doing this patch, I noticed that indirect PCI access on ixp4xx is broken since the PCIBIOS_MIN_MEM consolidateion, and this does not get fixed here.

Signed-off-by Arnd Bergmann <[email protected]>

arch/arm/mach-ixp4xx/avila-setup.c +2 -2

index 90e42e9..6cb33b9 100644
@@ -78,7 +78,7 @@ static struct resource avila_uart_resources[] = {
 static struct plat_serial8250_port avila_uart_data[] = {
 	{
 		.mapbase	= IXP4XX_UART1_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART1,
 		.flags		= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST,
 		.iotype		= UPIO_MEM,

@@ -87,7 +87,7 @@ static struct plat_serial8250_port avila_uart_data[] = {
 	},
 	{
 		.mapbase	= IXP4XX_UART2_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART2,
 		.flags		= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST,
 		.iotype		= UPIO_MEM,

arch/arm/mach-ixp4xx/common-pci.c +27 ~~-26~~

144 lines not on this page · raw message

arch/arm/mach-ixp4xx/common.c +32 ~~-30~~

178 lines not on this page · raw message

arch/arm/mach-ixp4xx/coyote-setup.c +4 -5

index 1b83110..02a1e98 100644
@@ -61,7 +61,7 @@ static struct resource coyote_uart_resource = {
 static struct plat_serial8250_port coyote_uart_data[] = {
 	{
 		.mapbase	= IXP4XX_UART2_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART2,
 		.flags		= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST,
 		.iotype		= UPIO_MEM,

@@ -93,12 +93,11 @@ static void __init coyote_init(void)
 	coyote_flash_resource.start = IXP4XX_EXP_BUS_BASE(0);
 	coyote_flash_resource.end = IXP4XX_EXP_BUS_BASE(0) + SZ_32M - 1;
 
-	*IXP4XX_EXP_CS0 |= IXP4XX_FLASH_WRITABLE;
-	*IXP4XX_EXP_CS1 = *IXP4XX_EXP_CS0;
+	writel_relaxed(readl_relaxed(IXP4XX_EXP_CS0) | IXP4XX_FLASH_WRITABLE, IXP4XX_EXP_CS0);
+	writel_relaxed(readl_relaxed(IXP4XX_EXP_CS0), IXP4XX_EXP_CS1);
 
 	if (machine_is_ixdpg425()) {
-		coyote_uart_data[0].membase =
-			(char*)(IXP4XX_UART1_BASE_VIRT + REG_OFFSET);
+		coyote_uart_data[0].membase = IXP4XX_UART1_BASE_VIRT + REG_OFFSET;
 		coyote_uart_data[0].mapbase = IXP4XX_UART1_BASE_PHYS;
 		coyote_uart_data[0].irq = IRQ_IXP4XX_UART1;
 	}

arch/arm/mach-ixp4xx/dsmg600-setup.c +4 -3

index 97a0af8..2f24df1 100644
@@ -24,6 +24,7 @@
 #include <linux/serial_8250.h>
 #include <linux/leds.h>
 #include <linux/reboot.h>
+#include <linux/io.h>
 #include <linux/i2c.h>
 #include <linux/i2c-gpio.h>

@@ -124,7 +125,7 @@ static struct resource dsmg600_uart_resources[] = {
 static struct plat_serial8250_port dsmg600_uart_data[] = {
 	{
 		.mapbase	= IXP4XX_UART1_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART1,
 		.flags		= UPF_BOOT_AUTOCONF,
 		.iotype		= UPIO_MEM,

@@ -133,7 +134,7 @@ static struct plat_serial8250_port dsmg600_uart_data[] = {
 	},
 	{
 		.mapbase	= IXP4XX_UART2_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART2,
 		.flags		= UPF_BOOT_AUTOCONF,
 		.iotype		= UPIO_MEM,

@@ -235,7 +236,7 @@ static void __init dsmg600_init(void)
 	ixp4xx_sys_init();
 
 	/* Make sure that GPIO14 and GPIO15 are not used as clocks */
-	*IXP4XX_GPIO_GPCLKR = 0;
+	writel_relaxed(0, IXP4XX_GPIO_GPCLKR);
 
 	dsmg600_flash_resource.start = IXP4XX_EXP_BUS_BASE(0);
 	dsmg600_flash_resource.end =

arch/arm/mach-ixp4xx/fsg-setup.c +5 -5

index 9175a25..d3b0737 100644
@@ -89,7 +89,7 @@ static struct resource fsg_uart_resources[] = {
 static struct plat_serial8250_port fsg_uart_data[] = {
 	{
 		.mapbase	= IXP4XX_UART1_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART1,
 		.flags		= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST,
 		.iotype		= UPIO_MEM,

@@ -98,7 +98,7 @@ static struct plat_serial8250_port fsg_uart_data[] = {
 	},
 	{
 		.mapbase	= IXP4XX_UART2_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART2,
 		.flags		= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST,
 		.iotype		= UPIO_MEM,

@@ -190,11 +190,11 @@ static void __init fsg_init(void)
 	fsg_flash_resource.end =
 		IXP4XX_EXP_BUS_BASE(0) + ixp4xx_exp_bus_size - 1;
 
-	*IXP4XX_EXP_CS0 |= IXP4XX_FLASH_WRITABLE;
-	*IXP4XX_EXP_CS1 = *IXP4XX_EXP_CS0;
+	writel_relaxed(readl_relaxed(IXP4XX_EXP_CS0) | IXP4XX_FLASH_WRITABLE, IXP4XX_EXP_CS0);
+	writel_relaxed(readl_relaxed(IXP4XX_EXP_CS0), IXP4XX_EXP_CS1);
 
 	/* Configure CS2 for operation, 8bit and writable */
-	*IXP4XX_EXP_CS2 = 0xbfff0002;
+	writel_relaxed(0xbfff0002, IXP4XX_EXP_CS2);
 
 	i2c_register_board_info(0, fsg_i2c_board_info,
 				ARRAY_SIZE(fsg_i2c_board_info));

arch/arm/mach-ixp4xx/gateway7001-setup.c +3 -3

index 033c717..57a58ea 100644
@@ -55,7 +55,7 @@ static struct resource gateway7001_uart_resource = {
 static struct plat_serial8250_port gateway7001_uart_data[] = {
 	{
 		.mapbase	= IXP4XX_UART2_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART2,
 		.flags		= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST,
 		.iotype		= UPIO_MEM,

@@ -87,8 +87,8 @@ static void __init gateway7001_init(void)
 	gateway7001_flash_resource.start = IXP4XX_EXP_BUS_BASE(0);
 	gateway7001_flash_resource.end = IXP4XX_EXP_BUS_BASE(0) + SZ_32M - 1;
 
-	*IXP4XX_EXP_CS0 |= IXP4XX_FLASH_WRITABLE;
-	*IXP4XX_EXP_CS1 = *IXP4XX_EXP_CS0;
+	writel_relaxed(readl_relaxed(IXP4XX_EXP_CS0) | IXP4XX_FLASH_WRITABLE, IXP4XX_EXP_CS0);
+	writel_relaxed(readl_relaxed(IXP4XX_EXP_CS0), IXP4XX_EXP_CS1);
 
 	platform_add_devices(gateway7001_devices, ARRAY_SIZE(gateway7001_devices));
 }

arch/arm/mach-ixp4xx/goramo_mlr.c +1 -0

index b800a03..9e52b88 100644
@@ -11,6 +11,7 @@
 #include <linux/kernel.h>
 #include <linux/pci.h>
 #include <linux/serial_8250.h>
+#include <asm/system_info.h>
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/flash.h>

arch/arm/mach-ixp4xx/gtwx5715-setup.c +1 -1

index 18ebc6b..62e3dee 100644
@@ -107,7 +107,7 @@ static struct resource gtwx5715_uart_resources[] = {
 static struct plat_serial8250_port gtwx5715_uart_platform_data[] = {
 	{
 	.mapbase	= IXP4XX_UART2_BASE_PHYS,
-	.membase	= (char *)IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
+	.membase	= IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
 	.irq		= IRQ_IXP4XX_UART2,
 	.flags		= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST,
 	.iotype		= UPIO_MEM,

arch/arm/mach-ixp4xx/include/mach/io.h +4 -4

index 5cf30d1..795988d 100644
@@ -75,7 +75,7 @@ static inline void __indirect_writeb(u8 value, volatile void __iomem *p)
 	u32 n, byte_enables, data;
 
 	if (!is_pci_memory(addr)) {
-		__raw_writeb(value, addr);
+		__raw_writeb(value, p);
 		return;
 	}

@@ -98,7 +98,7 @@ static inline void __indirect_writew(u16 value, volatile void __iomem *p)
 	u32 n, byte_enables, data;
 
 	if (!is_pci_memory(addr)) {
-		__raw_writew(value, addr);
+		__raw_writew(value, p);
 		return;
 	}

@@ -140,7 +140,7 @@ static inline unsigned char __indirect_readb(const volatile void __iomem *p)
 	u32 n, byte_enables, data;
 
 	if (!is_pci_memory(addr))
-		return __raw_readb(addr);
+		return __raw_readb(p);
 
 	n = addr % 4;
 	byte_enables = (0xf & ~BIT(n)) << IXP4XX_PCI_NP_CBE_BESL;

@@ -163,7 +163,7 @@ static inline unsigned short __indirect_readw(const volatile void __iomem *p)
 	u32 n, byte_enables, data;
 
 	if (!is_pci_memory(addr))
-		return __raw_readw(addr);
+		return __raw_readw(p);
 
 	n = addr % 4;
 	byte_enables = (0xf & ~(BIT(n) | BIT(n+1))) << IXP4XX_PCI_NP_CBE_BESL;

arch/arm/mach-ixp4xx/include/mach/ixp4xx-regs.h +6 -6

index eb68b61..2522ab0 100644
@@ -92,7 +92,7 @@
 /*
  * Expansion Bus Controller registers.
  */
-#define IXP4XX_EXP_REG(x) ((volatile u32 __iomem *)(IXP4XX_EXP_CFG_BASE_VIRT+(x)))
+#define IXP4XX_EXP_REG(x) (IXP4XX_EXP_CFG_BASE_VIRT+(x))
 
 #define IXP4XX_EXP_CS0      IXP4XX_EXP_REG(IXP4XX_EXP_CS0_OFFSET)
 #define IXP4XX_EXP_CS1      IXP4XX_EXP_REG(IXP4XX_EXP_CS1_OFFSET)

@@ -182,7 +182,7 @@
  * Interrupt Controller Register Definitions.
  */
 
-#define IXP4XX_INTC_REG(x) ((volatile u32 *)(IXP4XX_INTC_BASE_VIRT+(x)))
+#define IXP4XX_INTC_REG(x) (IXP4XX_INTC_BASE_VIRT+(x))
 
 #define IXP4XX_ICPR	IXP4XX_INTC_REG(IXP4XX_ICPR_OFFSET)
 #define IXP4XX_ICMR     IXP4XX_INTC_REG(IXP4XX_ICMR_OFFSET)

@@ -215,7 +215,7 @@
  * GPIO Register Definitions.
  * [Only perform 32bit reads/writes]
  */
-#define IXP4XX_GPIO_REG(x) ((volatile u32 *)(IXP4XX_GPIO_BASE_VIRT+(x)))
+#define IXP4XX_GPIO_REG(x) (IXP4XX_GPIO_BASE_VIRT+(x))
 
 #define IXP4XX_GPIO_GPOUTR	IXP4XX_GPIO_REG(IXP4XX_GPIO_GPOUTR_OFFSET)
 #define IXP4XX_GPIO_GPOER       IXP4XX_GPIO_REG(IXP4XX_GPIO_GPOER_OFFSET)

@@ -261,7 +261,7 @@
  * Operating System Timer Register Definitions.
  */
 
-#define IXP4XX_TIMER_REG(x) ((volatile u32 *)(IXP4XX_TIMER_BASE_VIRT+(x)))
+#define IXP4XX_TIMER_REG(x) (IXP4XX_TIMER_BASE_VIRT+(x))
 
 #define IXP4XX_OSTS	IXP4XX_TIMER_REG(IXP4XX_OSTS_OFFSET)
 #define IXP4XX_OST1	IXP4XX_TIMER_REG(IXP4XX_OST1_OFFSET)

@@ -323,7 +323,7 @@
 /*
  * PCI Control/Status Registers
  */
-#define IXP4XX_PCI_CSR(x) ((volatile u32 *)(IXP4XX_PCI_CFG_BASE_VIRT+(x)))
+#define IXP4XX_PCI_CSR(x) (IXP4XX_PCI_CFG_BASE_VIRT+(x))
 
 #define PCI_NP_AD               IXP4XX_PCI_CSR(PCI_NP_AD_OFFSET)
 #define PCI_NP_CBE              IXP4XX_PCI_CSR(PCI_NP_CBE_OFFSET)

@@ -411,7 +411,7 @@
  * IXP4XX_ naming convetions.
  *
  */
-# define IXP4XX_USB_REG(x)       (*((volatile u32 *)(x)))
+# define IXP4XX_USB_REG(x)       (*((volatile u32 __iomem *)(x)))
 
 /* UDC Undocumented - Reserved1 */
 #define UDC_RES1	IXP4XX_USB_REG(IXP4XX_USB_BASE_VIRT+0x0004)

arch/arm/mach-ixp4xx/include/mach/platform.h +25 ~~-22~~

59 lines not on this page · raw message

arch/arm/mach-ixp4xx/include/mach/timex.h +1 -1

index c9e930f..0396d89 100644
@@ -3,7 +3,7 @@
  * 
  */
 
-#include <mach/hardware.h>
+#include <mach/ixp4xx-regs.h>
 
 /*
  * We use IXP425 General purpose timer for our timer needs, it runs at

arch/arm/mach-ixp4xx/ixdp425-setup.c +2 -2

index 108a9d3..399c5e3 100644
@@ -150,7 +150,7 @@ static struct resource ixdp425_uart_resources[] = {
 static struct plat_serial8250_port ixdp425_uart_data[] = {
 	{
 		.mapbase	= IXP4XX_UART1_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART1,
 		.flags		= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST,
 		.iotype		= UPIO_MEM,

@@ -159,7 +159,7 @@ static struct plat_serial8250_port ixdp425_uart_data[] = {
 	},
 	{
 		.mapbase	= IXP4XX_UART2_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART2,
 		.flags		= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST,
 		.iotype		= UPIO_MEM,

arch/arm/mach-ixp4xx/ixp4xx_qmgr.c +6 -6

index 852f7c9..f0e1169 100644
@@ -55,14 +55,14 @@ static irqreturn_t qmgr_irq1_a0(int irq, void *pdev)
 	u32 en_bitmap, src, stat;
 
 	/* ACK - it may clear any bits so don't rely on it */
-	__raw_writel(0xFFFFFFFF, &qmgr_regs->irqstat[0]);
+	writel_relaxed(0xFFFFFFFF, &qmgr_regs->irqstat[0]);
 
-	en_bitmap = qmgr_regs->irqen[0];
+	en_bitmap = readl_relaxed(&qmgr_regs->irqen[0]);
 	while (en_bitmap) {
 		i = __fls(en_bitmap); /* number of the last "low" queue */
 		en_bitmap &= ~BIT(i);
-		src = qmgr_regs->irqsrc[i >> 3];
-		stat = qmgr_regs->stat1[i >> 3];
+		src = readl_relaxed(&qmgr_regs->irqsrc[i >> 3]);
+		stat = readl_relaxed(&qmgr_regs->stat1[i >> 3]);
 		if (src & 4) /* the IRQ condition is inverted */
 			stat = ~stat;
 		if (stat & BIT(src & 3)) {

@@ -80,9 +80,9 @@ static irqreturn_t qmgr_irq2_a0(int irq, void *pdev)
 	u32 req_bitmap;
 
 	/* ACK - it may clear any bits so don't rely on it */
-	__raw_writel(0xFFFFFFFF, &qmgr_regs->irqstat[1]);
+	writel_relaxed(0xFFFFFFFF, &qmgr_regs->irqstat[1]);
 
-	req_bitmap = qmgr_regs->irqen[1] & qmgr_regs->statne_h;
+	req_bitmap = readl_relaxed(&qmgr_regs->irqen[1]) & readl_relaxed(&qmgr_regs->statne_h);
 	while (req_bitmap) {
 		i = __fls(req_bitmap); /* number of the last "high" queue */
 		req_bitmap &= ~BIT(i);

arch/arm/mach-ixp4xx/nas100d-setup.c +3 -3

index 33cb095..647d377 100644
@@ -129,7 +129,7 @@ static struct resource nas100d_uart_resources[] = {
 static struct plat_serial8250_port nas100d_uart_data[] = {
 	{
 		.mapbase	= IXP4XX_UART1_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART1,
 		.flags		= UPF_BOOT_AUTOCONF,
 		.iotype		= UPIO_MEM,

@@ -138,7 +138,7 @@ static struct plat_serial8250_port nas100d_uart_data[] = {
 	},
 	{
 		.mapbase	= IXP4XX_UART2_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART2,
 		.flags		= UPF_BOOT_AUTOCONF,
 		.iotype		= UPIO_MEM,

@@ -250,7 +250,7 @@ static void __init nas100d_init(void)
 	ixp4xx_sys_init();
 
 	/* gpio 14 and 15 are _not_ clocks */
-	*IXP4XX_GPIO_GPCLKR = 0;
+	writel_relaxed(0, IXP4XX_GPIO_GPCLKR);
 
 	nas100d_flash_resource.start = IXP4XX_EXP_BUS_BASE(0);
 	nas100d_flash_resource.end =

arch/arm/mach-ixp4xx/nslu2-setup.c +2 -2

index e2903fa..ada1175 100644
@@ -141,7 +141,7 @@ static struct resource nslu2_uart_resources[] = {
 static struct plat_serial8250_port nslu2_uart_data[] = {
 	{
 		.mapbase	= IXP4XX_UART1_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART1,
 		.flags		= UPF_BOOT_AUTOCONF,
 		.iotype		= UPIO_MEM,

@@ -150,7 +150,7 @@ static struct plat_serial8250_port nslu2_uart_data[] = {
 	},
 	{
 		.mapbase	= IXP4XX_UART2_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART2,
 		.flags		= UPF_BOOT_AUTOCONF,
 		.iotype		= UPIO_MEM,

arch/arm/mach-ixp4xx/omixp-setup.c +3 -2

index 158ddb7..6a0be72 100644
@@ -17,6 +17,7 @@
 #include <linux/serial_8250.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
+#include <linux/io.h>
 #ifdef CONFIG_LEDS_CLASS
 #include <linux/leds.h>
 #endif

@@ -125,7 +126,7 @@ static struct resource omixp_uart_resources[] = {
 static struct plat_serial8250_port omixp_uart_data[] = {
 	{
 		.mapbase	= IXP4XX_UART2_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART2,
 		.flags		= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST,
 		.iotype		= UPIO_MEM,

@@ -133,7 +134,7 @@ static struct plat_serial8250_port omixp_uart_data[] = {
 		.uartclk	= IXP4XX_UART_XTAL,
 	}, {
 		.mapbase	= IXP4XX_UART1_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART1,
 		.flags		= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST,
 		.iotype		= UPIO_MEM,

arch/arm/mach-ixp4xx/vulcan-setup.c +13 ~~-13~~

index 2798f43..846192d 100644
@@ -77,7 +77,7 @@ static struct resource vulcan_uart_resources[] = {
 static struct plat_serial8250_port vulcan_uart_data[] = {
 	[0] = {
 		.mapbase	= IXP4XX_UART1_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART1_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART1,
 		.flags		= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST,
 		.iotype		= UPIO_MEM,

@@ -86,7 +86,7 @@ static struct plat_serial8250_port vulcan_uart_data[] = {
 	},
 	[1] = {
 		.mapbase	= IXP4XX_UART2_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART2,
 		.flags		= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST,
 		.iotype		= UPIO_MEM,

@@ -190,46 +190,46 @@ static void __init vulcan_init(void)
 	/* Flash is spread over both CS0 and CS1 */
 	vulcan_flash_resource.start	 = IXP4XX_EXP_BUS_BASE(0);
 	vulcan_flash_resource.end	 = IXP4XX_EXP_BUS_BASE(0) + SZ_32M - 1;
-	*IXP4XX_EXP_CS0 = IXP4XX_EXP_BUS_CS_EN		|
+	writel_relaxed(IXP4XX_EXP_BUS_CS_EN		|
 			  IXP4XX_EXP_BUS_STROBE_T(3)	|
 			  IXP4XX_EXP_BUS_SIZE(0xF)	|
 			  IXP4XX_EXP_BUS_BYTE_RD16	|
-			  IXP4XX_EXP_BUS_WR_EN;
-	*IXP4XX_EXP_CS1 = *IXP4XX_EXP_CS0;
+			  IXP4XX_EXP_BUS_WR_EN, IXP4XX_EXP_CS0);
+	writel_relaxed(readl_relaxed(IXP4XX_EXP_CS0), IXP4XX_EXP_CS1);
 
 	/* SRAM on CS2, (256kB, 8bit, writable) */
 	vulcan_sram_resource.start	= IXP4XX_EXP_BUS_BASE(2);
 	vulcan_sram_resource.end	= IXP4XX_EXP_BUS_BASE(2) + SZ_256K - 1;
-	*IXP4XX_EXP_CS2 = IXP4XX_EXP_BUS_CS_EN		|
+	writel_relaxed(IXP4XX_EXP_BUS_CS_EN		|
 			  IXP4XX_EXP_BUS_STROBE_T(1)	|
 			  IXP4XX_EXP_BUS_HOLD_T(2)	|
 			  IXP4XX_EXP_BUS_SIZE(9)	|
 			  IXP4XX_EXP_BUS_SPLT_EN	|
 			  IXP4XX_EXP_BUS_WR_EN		|
-			  IXP4XX_EXP_BUS_BYTE_EN;
+			  IXP4XX_EXP_BUS_BYTE_EN, IXP4XX_EXP_CS2);
 
 	/* XR16L2551 on CS3 (Moto style, 512 bytes, 8bits, writable) */
 	vulcan_uart_resources[2].start	= IXP4XX_EXP_BUS_BASE(3);
 	vulcan_uart_resources[2].end	= IXP4XX_EXP_BUS_BASE(3) + 16 - 1;
 	vulcan_uart_data[2].mapbase	= vulcan_uart_resources[2].start;
 	vulcan_uart_data[3].mapbase	= vulcan_uart_data[2].mapbase + 8;
-	*IXP4XX_EXP_CS3 = IXP4XX_EXP_BUS_CS_EN		|
+	writel_relaxed(IXP4XX_EXP_BUS_CS_EN		|
 			  IXP4XX_EXP_BUS_STROBE_T(3)	|
 			  IXP4XX_EXP_BUS_CYCLES(IXP4XX_EXP_BUS_CYCLES_MOTOROLA)|
 			  IXP4XX_EXP_BUS_WR_EN		|
-			  IXP4XX_EXP_BUS_BYTE_EN;
+			  IXP4XX_EXP_BUS_BYTE_EN, IXP4XX_EXP_CS3);
 
 	/* GPIOS on CS4 (512 bytes, 8bits, writable) */
-	*IXP4XX_EXP_CS4 = IXP4XX_EXP_BUS_CS_EN		|
+	writel_relaxed(IXP4XX_EXP_BUS_CS_EN		|
 			  IXP4XX_EXP_BUS_WR_EN		|
-			  IXP4XX_EXP_BUS_BYTE_EN;
+			  IXP4XX_EXP_BUS_BYTE_EN, IXP4XX_EXP_CS4);
 
 	/* max6369 on CS5 (512 bytes, 8bits, writable) */
 	vulcan_max6369_resource.start	= IXP4XX_EXP_BUS_BASE(5);
 	vulcan_max6369_resource.end	= IXP4XX_EXP_BUS_BASE(5);
-	*IXP4XX_EXP_CS5 = IXP4XX_EXP_BUS_CS_EN		|
+	writel_relaxed(IXP4XX_EXP_BUS_CS_EN		|
 			  IXP4XX_EXP_BUS_WR_EN		|
-			  IXP4XX_EXP_BUS_BYTE_EN;
+			  IXP4XX_EXP_BUS_BYTE_EN, IXP4XX_EXP_CS5);
 
 	platform_add_devices(vulcan_devices, ARRAY_SIZE(vulcan_devices));
 }

arch/arm/mach-ixp4xx/wg302v2-setup.c +3 -3

index a785175..93f7948 100644
@@ -56,7 +56,7 @@ static struct resource wg302v2_uart_resource = {
 static struct plat_serial8250_port wg302v2_uart_data[] = {
 	{
 		.mapbase	= IXP4XX_UART2_BASE_PHYS,
-		.membase	= (char *)IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
+		.membase	= IXP4XX_UART2_BASE_VIRT + REG_OFFSET,
 		.irq		= IRQ_IXP4XX_UART2,
 		.flags		= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST,
 		.iotype		= UPIO_MEM,

@@ -88,8 +88,8 @@ static void __init wg302v2_init(void)
 	wg302v2_flash_resource.start = IXP4XX_EXP_BUS_BASE(0);
 	wg302v2_flash_resource.end = IXP4XX_EXP_BUS_BASE(0) + SZ_32M - 1;
 
-	*IXP4XX_EXP_CS0 |= IXP4XX_FLASH_WRITABLE;
-	*IXP4XX_EXP_CS1 = *IXP4XX_EXP_CS0;
+	writel_relaxed(readl_relaxed(IXP4XX_EXP_CS0) | IXP4XX_FLASH_WRITABLE, IXP4XX_EXP_CS0);
+	writel_relaxed(readl_relaxed(IXP4XX_EXP_CS0), IXP4XX_EXP_CS1);
 
 	platform_add_devices(wg302v2_devices, ARRAY_SIZE(wg302v2_devices));
 }

drivers/input/misc/ixp4xx-beeper.c +4 -4

index 50e2830..d6d17e5 100644
@@ -39,12 +39,12 @@ static void ixp4xx_spkr_control(unsigned int pin, unsigned int count)
 		gpio_line_config(pin, IXP4XX_GPIO_OUT);
 		gpio_line_set(pin, IXP4XX_GPIO_LOW);
 
-		*IXP4XX_OSRT2 = (count & ~IXP4XX_OST_RELOAD_MASK) | IXP4XX_OST_ENABLE;
+		writel_relaxed((count & ~IXP4XX_OST_RELOAD_MASK) | IXP4XX_OST_ENABLE, IXP4XX_OSRT2);
 	} else {
 		gpio_line_config(pin, IXP4XX_GPIO_IN);
 		gpio_line_set(pin, IXP4XX_GPIO_HIGH);
 
-		*IXP4XX_OSRT2 = 0;
+		writel_relaxed(0, IXP4XX_OSRT2);
 	}
 
 	spin_unlock_irqrestore(&beep_lock, flags);

@@ -79,10 +79,10 @@ static int ixp4xx_spkr_event(struct input_dev *dev, unsigned int type, unsigned
 static irqreturn_t ixp4xx_spkr_interrupt(int irq, void *dev_id)
 {
 	/* clear interrupt */
-	*IXP4XX_OSST = IXP4XX_OSST_TIMER_2_PEND;
+	writel_relaxed(IXP4XX_OSST_TIMER_2_PEND, IXP4XX_OSST);
 
 	/* flip the beeper output */
-	*IXP4XX_GPIO_GPOUTR ^= (1 << (unsigned int) dev_id);
+	writel_relaxed(readl_relaxed(IXP4XX_GPIO_GPOUTR) ^ (1 << (unsigned int) dev_id), IXP4XX_GPIO_GPOUTR);
 
 	return IRQ_HANDLED;
 }

drivers/watchdog/ixp4xx_wdt.c +9 -9

index 5580b4f..f5ade39 100644
@@ -41,20 +41,20 @@ static DEFINE_SPINLOCK(wdt_lock);
 static void wdt_enable(void)
 {
 	spin_lock(&wdt_lock);
-	*IXP4XX_OSWK = IXP4XX_WDT_KEY;
-	*IXP4XX_OSWE = 0;
-	*IXP4XX_OSWT = WDT_TICK_RATE * heartbeat;
-	*IXP4XX_OSWE = IXP4XX_WDT_COUNT_ENABLE | IXP4XX_WDT_RESET_ENABLE;
-	*IXP4XX_OSWK = 0;
+	writel_relaxed(IXP4XX_WDT_KEY, IXP4XX_OSWK);
+	writel_relaxed(0, IXP4XX_OSWE);
+	writel_relaxed(WDT_TICK_RATE * heartbeat, IXP4XX_OSWT);
+	writel_relaxed(IXP4XX_WDT_COUNT_ENABLE | IXP4XX_WDT_RESET_ENABLE, IXP4XX_OSWE);
+	writel_relaxed(0, IXP4XX_OSWK);
 	spin_unlock(&wdt_lock);
 }
 
 static void wdt_disable(void)
 {
 	spin_lock(&wdt_lock);
-	*IXP4XX_OSWK = IXP4XX_WDT_KEY;
-	*IXP4XX_OSWE = 0;
-	*IXP4XX_OSWK = 0;
+	writel_relaxed(IXP4XX_WDT_KEY, IXP4XX_OSWK);
+	writel_relaxed(0, IXP4XX_OSWE);
+	writel_relaxed(0, IXP4XX_OSWK);
 	spin_unlock(&wdt_lock);
 }

@@ -181,7 +181,7 @@ static int __init ixp4xx_wdt_init(void)
 
 		return -ENODEV;
 	}
-	boot_status = (*IXP4XX_OSST & IXP4XX_OSST_TIMER_WARM_RESET) ?
+	boot_status = (readl_relaxed(IXP4XX_OSST) & IXP4XX_OSST_TIMER_WARM_RESET) ?
 			WDIOF_CARDRESET : 0;
 	ret = misc_register(&ixp4xx_wdt_miscdev);
 	if (ret == 0)

Alexander Shiyan

author

2012-12-02 06:02 UTC #7 lore raw

Quoted 53 lines · Jason Cooper

Quoted 28 lines · Arnd Bergmann
Quoted 15 lines · Alexander Shiyan
Review
Quoted 13 lines
(quoted diff) +13 -0
+       switch (d->irq) {
+       case IRQ_IXP4XX_TIMER1:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_1_PEND;
+               break;
+       case IRQ_IXP4XX_TIMER2:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_2_PEND;
+               break;
+       case IRQ_IXP4XX_TIMESTAMP:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_TS_PEND;
+               break;
+       case IRQ_IXP4XX_WDOG:
+               *IXP4XX_OSST = IXP4XX_OSST_TIMER_WDOG_PEND;
+               break;
Quoted 4 lines
Since you are touching these lines, it probably makes sense to convert them to use writel_relaxed() in the process. Dereferencing a volatile pointer in order to do MMIO is strongly discouraged, see Documentation/volatile-considered-harmful.txt
Arnd,
I took a quick look at the ixp4xx code when I saw this. It appears the entire sub-arch is written this way :-( Perhaps it would be better to do a cleanup patch before this one? I didn't mention it in my initial comment because it looks like quite a bit of work.
Quoted 6 lines
In either case, it's all cleanup, so it shouldn't cause a dependency headache.
Alexander, if you're so inclined, a cleanup series would be much appreciated. If you don't have the time, no problem, just make the changes suggested by Arnd and I and we'll get to the cleanup eventually.
I got curious to how hard this would be and ended up with a patch.
Arnd
8<--------
[PATCH] ARM: ixp4xx: use proper __iomem annotations consistently
The ixp4xx platform on ARM is one of the remaining locations still using direct pointer dereferences for MMIO access. This patch should convert all known instances to use readl_relaxed/writel_relaxed.
I could not find a nice solution for mach/hardware.h, which is included by mach/io.h indirectly but actually requires MMIO accesses as defined in asm/io.h. Using a macro as a workaround helps, but the better solution in the long run would be to have a proper gpiolib driver rather than using a private API to do GPIO.

Quoted 7 lines

I did not touch the definitions used in drivers/usb/gadget/pxa25x_udc.c, they are shared with the pxa platform and ever more screwed up than the other ones in ixp4xx that I fixed.
In the process of doing this patch, I noticed that indirect PCI access on ixp4xx is broken since the PCIBIOS_MIN_MEM consolidateion, and this does not get fixed here.

Nice work! I changed my patch and maybe do another one for this modified version of common.c. Only one comment about this patch is below.

Quoted 3 lines

Signed-off-by Arnd Bergmann <[email protected]>

...

arch/arm/mach-ixp4xx/include/mach/ixp4xx-regs.h line 95

Quoted 10 lines

arch/arm/mach-ixp4xx/include/mach/ixp4xx-regs.h +1 -1

index eb68b61..2522ab0 100644
@@ -92,7 +92,7 @@
 /*
  * Expansion Bus Controller registers.
  */
-#define IXP4XX_EXP_REG(x) ((volatile u32 __iomem *)(IXP4XX_EXP_CFG_BASE_VIRT+(x)))
+#define IXP4XX_EXP_REG(x) (IXP4XX_EXP_CFG_BASE_VIRT+(x))

Modify this to:
#define IXP4XX_EXP_REG(x) IOMEM(IXP4XX_EXP_CFG_BASE_VIRT+(x)) to avoid compiler warnings.

---

Arnd Bergmann

2012-12-02 13:13 UTC #8 lore raw

PATCH 1/1 ARM: ixp4xx: Add "ask" handler for timer interrupts

Quoted 5 lines · Alexander Shiyan

Quoted 5 lines
arch/arm/mach-ixp4xx/include/mach/ixp4xx-regs.h +0 -0
index eb68b61..2522ab0 100644
@@ -92,7 +92,7 @@

Quoted 9 lines

arch/arm/mach-ixp4xx/include/mach/ixp4xx-regs.h line 95
Quoted 5 lines
arch/arm/mach-ixp4xx/include/mach/ixp4xx-regs.h +1 -1
 /*
  * Expansion Bus Controller registers.
  */
-#define IXP4XX_EXP_REG(x) ((volatile u32 __iomem *)(IXP4XX_EXP_CFG_BASE_VIRT+(x)))
+#define IXP4XX_EXP_REG(x) (IXP4XX_EXP_CFG_BASE_VIRT+(x))
Modify this to:
#define IXP4XX_EXP_REG(x) IOMEM(IXP4XX_EXP_CFG_BASE_VIRT+(x)) to avoid compiler warnings.

Hmm, not sure what went wrong here. Actually the definition of IXP4XX_EXP_CFG_BASE_VIRT itself should use the IOMEM() macro, like all the other IXP4XX_*_BASE_VIRT do.

Arnd

Alexander Shiyan

author

2012-12-02 13:55 UTC #9 lore raw

Quoted 16 lines · Alexander Shiyan

Quoted 5 lines
Quoted 5 lines
arch/arm/mach-ixp4xx/include/mach/ixp4xx-regs.h +0 -0
index eb68b61..2522ab0 100644
@@ -92,7 +92,7 @@
Quoted 9 lines
arch/arm/mach-ixp4xx/include/mach/ixp4xx-regs.h line 95
Quoted 5 lines
arch/arm/mach-ixp4xx/include/mach/ixp4xx-regs.h +1 -1
 /*
  * Expansion Bus Controller registers.
  */
-#define IXP4XX_EXP_REG(x) ((volatile u32 __iomem *)(IXP4XX_EXP_CFG_BASE_VIRT+(x)))
+#define IXP4XX_EXP_REG(x) (IXP4XX_EXP_CFG_BASE_VIRT+(x))
Modify this to:
#define IXP4XX_EXP_REG(x) IOMEM(IXP4XX_EXP_CFG_BASE_VIRT+(x)) to avoid compiler warnings.

Quoted 3 lines

Hmm, not sure what went wrong here. Actually the definition of IXP4XX_EXP_CFG_BASE_VIRT itself should use the IOMEM() macro, like all the other IXP4XX_*_BASE_VIRT do.

But without this is produce warnings:
CC arch/arm/common/dmabounce.o
In file included from arch/arm/mach-ixp4xx/include/mach/hardware.h:30,

                 from arch/arm/mach-ixp4xx/include/mach/io.h:18,
                 from /home/git/linux-next/arch/arm/include/asm/io.h:180,
                 from include/linux/scatterlist.h:10,
                 from include/linux/dma-mapping.h:9,
                 from arch/arm/common/dmabounce.c:30:

arch/arm/mach-ixp4xx/include/mach/cpu.h: In function 'ixp4xx_read_feature_bits':
arch/arm/mach-ixp4xx/include/mach/cpu.h:41: warning: passing argument 1 of '__raw_readl' makes pointer from integer without a cast arch/arm/mach-ixp4xx/include/mach/cpu.h: In function 'ixp4xx_write_feature_bits':
arch/arm/mach-ixp4xx/include/mach/cpu.h:55: warning: passing argument 2 of '__raw_writel' makes pointer from integer without a cast

---

Arnd Bergmann

2012-12-03 23:40 UTC #10 lore raw

PATCH 1/1 ARM: ixp4xx: Add "ask" handler for timer interrupts

Quoted 19 lines · Alexander Shiyan

Quoted 10 lines
Quoted 6 lines
Review
Quoted 2 lines
(quoted diff) +1 -1
-#define IXP4XX_EXP_REG(x) ((volatile u32 __iomem *)(IXP4XX_EXP_CFG_BASE_VIRT+(x)))
+#define IXP4XX_EXP_REG(x) (IXP4XX_EXP_CFG_BASE_VIRT+(x))
Modify this to:
#define IXP4XX_EXP_REG(x) IOMEM(IXP4XX_EXP_CFG_BASE_VIRT+(x)) to avoid compiler warnings.
Hmm, not sure what went wrong here. Actually the definition of IXP4XX_EXP_CFG_BASE_VIRT itself should use the IOMEM() macro, like all the other IXP4XX_*_BASE_VIRT do.
But without this is produce warnings:
CC arch/arm/common/dmabounce.o
In file included from arch/arm/mach-ixp4xx/include/mach/hardware.h:30,
                 from arch/arm/mach-ixp4xx/include/mach/io.h:18,
                 from /home/git/linux-next/arch/arm/include/asm/io.h:180,
                 from include/linux/scatterlist.h:10,
                 from include/linux/dma-mapping.h:9,
                 from arch/arm/common/dmabounce.c:30:

Quoted 5 lines

arch/arm/mach-ixp4xx/include/mach/cpu.h: In function 'ixp4xx_read_feature_bits':
arch/arm/mach-ixp4xx/include/mach/cpu.h:41: warning: passing argument 1 of '__raw_readl' makes pointer from integer without a cast arch/arm/mach-ixp4xx/include/mach/cpu.h: In function 'ixp4xx_write_feature_bits':
arch/arm/mach-ixp4xx/include/mach/cpu.h:55: warning: passing argument 2 of '__raw_writel' makes pointer from integer without a cast

I understand what the problem is, my point was that a better solution is to do

arch/arm/mach-ixp4xx/include/mach/ixp4xx-regs.h +1 -1

index c5bae9c..daf5c4e 100644
@@ -65,7 +65,7 @@
  * Expansion BUS Configuration registers
  */
 #define IXP4XX_EXP_CFG_BASE_PHYS	0xC4000000
-#define IXP4XX_EXP_CFG_BASE_VIRT	0xFEF14000
+#define IXP4XX_EXP_CFG_BASE_VIRT	IOMEM(0xFEF14000)
 #define IXP4XX_EXP_CFG_REGION_SIZE	0x00001000
 
 #define IXP4XX_EXP_CS0_OFFSET	0x00

instead of your suggested fix, because that matches how all the other _VIRT macros in that file are defined. I found now that it was correct until recently, but b7b23db72f accidentally broke it while fixing the MMIO location.

Arnd

Alexander Shiyan

author

2012-12-02 08:09 UTC #11 lore raw

PATCH 1/2 ARM: ixp4xx: Moving the timer flags control in ixp4xx_irq_ack() procedure

In reply to Arnd Bergmann

Rather than modify the EOI flags directly in the timer interrupt, let's deal with these flags in the ixp4xx_irq_ack() procedure.

Signed-off-by Alexander Shiyan <[email protected]>

---
 arch/arm/mach-ixp4xx/common.c      |   35 ++++++++++++++++++++++++++---------
 drivers/input/misc/ixp4xx-beeper.c |    3 ---
 2 files changed, 26 insertions(+), 12 deletions(-)

arch/arm/mach-ixp4xx/common.c +26 -9

index 7443e1d..8e2a3b8 100644
@@ -204,10 +204,28 @@ static void ixp4xx_irq_mask(struct irq_data *d)
 
 static void ixp4xx_irq_ack(struct irq_data *d)
 {
-	int line = (d->irq < 32) ? irq2gpio[d->irq] : -1;
+	int line;
 
-	if (line >= 0)
-		writel_relaxed(1 << line, IXP4XX_GPIO_GPISR);
+	switch (d->irq) {
+	case IRQ_IXP4XX_TIMER1:
+		writel_relaxed(IXP4XX_OSST_TIMER_1_PEND, IXP4XX_OSST);
+		break;
+	case IRQ_IXP4XX_TIMER2:
+		writel_relaxed(IXP4XX_OSST_TIMER_2_PEND, IXP4XX_OSST);
+		break;
+	case IRQ_IXP4XX_TIMESTAMP:
+		writel_relaxed(IXP4XX_OSST_TIMER_TS_PEND, IXP4XX_OSST);
+		break;
+	case IRQ_IXP4XX_WDOG:
+		writel_relaxed(IXP4XX_OSST_TIMER_WDOG_PEND, IXP4XX_OSST);
+		break;
+	default:
+		line = (d->irq < 32) ? irq2gpio[d->irq] : -1;
+
+		if (line >= 0)
+			writel_relaxed(1 << line, IXP4XX_GPIO_GPISR);
+		break;
+	}
 }
 
 /*

@@ -249,6 +267,11 @@ void __init ixp4xx_init_irq(void)
 	/* Disable all interrupt */
 	writel_relaxed(0, IXP4XX_ICMR);
 
+	/* Clear Timer Pending Interrupts */
+	writel_relaxed(IXP4XX_OSST_TIMER_1_PEND | IXP4XX_OSST_TIMER_2_PEND |
+		       IXP4XX_OSST_TIMER_TS_PEND | IXP4XX_OSST_TIMER_WDOG_PEND,
+		       IXP4XX_OSST);
+
 	if (cpu_is_ixp46x() || cpu_is_ixp43x()) {
 		/* Route upper 32 sources to IRQ instead of FIQ */
 		writel_relaxed(0, IXP4XX_ICLR2);

@@ -276,9 +299,6 @@ static irqreturn_t ixp4xx_timer_interrupt(int irq, void *dev_id)
 {
 	struct clock_event_device *evt = dev_id;
 
-	/* Clear Pending Interrupt by writing '1' to it */
-	writel_relaxed(IXP4XX_OSST_TIMER_1_PEND, IXP4XX_OSST);
-
 	evt->event_handler(evt);
 
 	return IRQ_HANDLED;

@@ -296,9 +316,6 @@ void __init ixp4xx_timer_init(void)
 	/* Reset/disable counter */
 	writel_relaxed(0, IXP4XX_OSRT1);
 
-	/* Clear Pending Interrupt by writing '1' to it */
-	writel_relaxed(IXP4XX_OSST_TIMER_1_PEND, IXP4XX_OSST);
-
 	/* Reset time-stamp counter */
 	writel_relaxed(0, IXP4XX_OSTS);

drivers/input/misc/ixp4xx-beeper.c +0 -3

index eae50fe..6b40950 100644
@@ -78,9 +78,6 @@ static int ixp4xx_spkr_event(struct input_dev *dev, unsigned int type, unsigned
 
 static irqreturn_t ixp4xx_spkr_interrupt(int irq, void *dev_id)
 {
-	/* clear interrupt */
-	writel_relaxed(IXP4XX_OSST_TIMER_2_PEND, IXP4XX_OSST);
-
 	/* flip the beeper output */
 	writel_relaxed(readl_relaxed(IXP4XX_GPIO_GPOUTR) ^ (1 << (unsigned int) dev_id), IXP4XX_GPIO_GPOUTR);

Signature · 1 line

1.7.8.6

Alexander Shiyan

author

2012-12-02 08:09 UTC #12 lore raw

PATCH 2/2 ARM: ixp4xx: Using gpiolib rather than a private GPIO API

Signed-off-by Alexander Shiyan <[email protected]>

---
 arch/arm/mach-ixp4xx/common.c                |   29 ++++++++++-----
 arch/arm/mach-ixp4xx/dsmg600-setup.c         |    9 ++---
 arch/arm/mach-ixp4xx/goramo_mlr.c            |   31 ++++++++--------
 arch/arm/mach-ixp4xx/include/mach/platform.h |   38 --------------------
 arch/arm/mach-ixp4xx/ixdp425-setup.c         |    6 ++--
 arch/arm/mach-ixp4xx/nas100d-setup.c         |    9 ++---
 arch/arm/mach-ixp4xx/nslu2-setup.c           |    5 +--
 drivers/input/misc/ixp4xx-beeper.c           |   49 ++++++++++++++------------
 drivers/ptp/ptp_ixp46x.c                     |   31 +++++++++++-----
 9 files changed, 91 insertions(+), 116 deletions(-)

arch/arm/mach-ixp4xx/common.c +19 ~~-10~~

index 8e2a3b8..2039051 100644
@@ -189,7 +189,7 @@ static int ixp4xx_set_irq_type(struct irq_data *d, unsigned int type)
 			int_reg);
 
 	/* Configure the line as an input */
-	gpio_line_config(irq2gpio[d->irq], IXP4XX_GPIO_IN);
+	gpio_direction_input(line);
 
 	return 0;
 }

@@ -401,7 +401,8 @@ EXPORT_SYMBOL(ixp4xx_exp_bus_size);
 
 static int ixp4xx_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
 {
-	gpio_line_config(gpio, IXP4XX_GPIO_IN);
+	writel_relaxed(readl_relaxed(IXP4XX_GPIO_GPOER) | (1 << gpio),
+		       IXP4XX_GPIO_GPOER);
 
 	return 0;
 }

@@ -409,25 +410,33 @@ static int ixp4xx_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
 static int ixp4xx_gpio_direction_output(struct gpio_chip *chip, unsigned gpio,
 					int level)
 {
-	gpio_line_set(gpio, level);
-	gpio_line_config(gpio, IXP4XX_GPIO_OUT);
+	u32 outval = readl_relaxed(IXP4XX_GPIO_GPOUTR) & ~(1 << gpio);
+
+	if (level)
+		outval |= 1 << gpio;
+
+	writel_relaxed(outval, IXP4XX_GPIO_GPOUTR);
+
+	writel_relaxed(readl_relaxed(IXP4XX_GPIO_GPOER) & ~(1 << gpio),
+		       IXP4XX_GPIO_GPOER);
 
 	return 0;
 }
 
 static int ixp4xx_gpio_get_value(struct gpio_chip *chip, unsigned gpio)
 {
-	int value;
-
-	gpio_line_get(gpio, &value);
-
-	return value;
+	return !!(readl_relaxed(IXP4XX_GPIO_GPINR) & (1 << gpio));
 }
 
 static void ixp4xx_gpio_set_value(struct gpio_chip *chip, unsigned gpio,
 				  int value)
 {
-	gpio_line_set(gpio, value);
+	u32 outval = readl_relaxed(IXP4XX_GPIO_GPOUTR) & ~(1 << gpio);
+
+	if (value)
+		outval |= 1 << gpio;
+
+	writel_relaxed(outval, IXP4XX_GPIO_GPOUTR);
 }
 
 static struct gpio_chip ixp4xx_gpio_chip = {

arch/arm/mach-ixp4xx/dsmg600-setup.c +3 -6

index 2f24df1..f6a80dc 100644
@@ -161,10 +161,7 @@ static struct platform_device *dsmg600_devices[] __initdata = {
 static void dsmg600_power_off(void)
 {
 	/* enable the pwr cntl gpio */
-	gpio_line_config(DSMG600_PO_GPIO, IXP4XX_GPIO_OUT);
-
-	/* poweroff */
-	gpio_line_set(DSMG600_PO_GPIO, IXP4XX_GPIO_HIGH);
+	gpio_direction_output(DSMG600_PO_GPIO, 1);
 }
 
 /* This is used to make sure the power-button pusher is serious.  The button

@@ -201,7 +198,7 @@ static void dsmg600_power_handler(unsigned long data)
 			ctrl_alt_del();
 
 			/* Change the state of the power LED to "blink" */
-			gpio_line_set(DSMG600_LED_PWR_GPIO, IXP4XX_GPIO_LOW);
+			gpio_set_value(DSMG600_LED_PWR_GPIO, 0);
 		} else {
 			power_button_countdown = PBUTTON_HOLDDOWN_COUNT;
 		}

@@ -269,7 +266,7 @@ static void __init dsmg600_init(void)
 	 */
 
 	/* Make sure that the power button GPIO is set up as an input */
-	gpio_line_config(DSMG600_PB_GPIO, IXP4XX_GPIO_IN);
+	gpio_direction_input(DSMG600_PB_GPIO);
 
 	/* Set the initial value for the power button IRQ handler */
 	power_button_countdown = PBUTTON_HOLDDOWN_COUNT;

arch/arm/mach-ixp4xx/goramo_mlr.c +15 ~~-16~~

index b81aa79..b80a256 100644
@@ -8,6 +8,7 @@
 #include <linux/i2c-gpio.h>
 #include <linux/io.h>
 #include <linux/irq.h>
+#include <linux/gpio.h>
 #include <linux/kernel.h>
 #include <linux/pci.h>
 #include <linux/serial_8250.h>

@@ -80,19 +81,19 @@ static u8 control_value;
 
 static void set_scl(u8 value)
 {
-	gpio_line_set(GPIO_SCL, !!value);
+	gpio_set_value(GPIO_SCL, value);
 	udelay(3);
 }
 
 static void set_sda(u8 value)
 {
-	gpio_line_set(GPIO_SDA, !!value);
+	gpio_set_value(GPIO_SDA, value);
 	udelay(3);
 }
 
 static void set_str(u8 value)
 {
-	gpio_line_set(GPIO_STR, !!value);
+	gpio_set_value(GPIO_STR, value);
 	udelay(3);
 }

@@ -109,8 +110,8 @@ static void output_control(void)
 {
 	int i;
 
-	gpio_line_config(GPIO_SCL, IXP4XX_GPIO_OUT);
-	gpio_line_config(GPIO_SDA, IXP4XX_GPIO_OUT);
+	gpio_direction_output(GPIO_SCL, 1);
+	gpio_direction_output(GPIO_SDA, 1);
 
 	for (i = 0; i < 8; i++) {
 		set_scl(0);

@@ -153,7 +154,7 @@ static int hss_set_clock(int port, unsigned int clock_type)
 static irqreturn_t hss_dcd_irq(int irq, void *pdev)
 {
 	int i, port = (irq == IXP4XX_GPIO_IRQ(GPIO_HSS1_DCD_N));
-	gpio_line_get(port ? GPIO_HSS1_DCD_N : GPIO_HSS0_DCD_N, &i);
+	i = gpio_get_value(port ? GPIO_HSS1_DCD_N : GPIO_HSS0_DCD_N);
 	set_carrier_cb_tab[port](pdev, !i);
 	return IRQ_HANDLED;
 }

@@ -169,7 +170,7 @@ static int hss_open(int port, void *pdev,
 	else
 		irq = IXP4XX_GPIO_IRQ(GPIO_HSS1_DCD_N);
 
-	gpio_line_get(port ? GPIO_HSS1_DCD_N : GPIO_HSS0_DCD_N, &i);
+	i = gpio_get_value(port ? GPIO_HSS1_DCD_N : GPIO_HSS0_DCD_N);
 	set_carrier_cb(pdev, !i);
 
 	set_carrier_cb_tab[!!port] = set_carrier_cb;

@@ -182,7 +183,7 @@ static int hss_open(int port, void *pdev,
 
 	set_control(port ? CONTROL_HSS1_DTR_N : CONTROL_HSS0_DTR_N, 0);
 	output_control();
-	gpio_line_set(port ? GPIO_HSS1_RTS_N : GPIO_HSS0_RTS_N, 0);
+	gpio_set_value(port ? GPIO_HSS1_RTS_N : GPIO_HSS0_RTS_N, 0);
 	return 0;
 }

@@ -194,7 +195,7 @@ static void hss_close(int port, void *pdev)
 
 	set_control(port ? CONTROL_HSS1_DTR_N : CONTROL_HSS0_DTR_N, 1);
 	output_control();
-	gpio_line_set(port ? GPIO_HSS1_RTS_N : GPIO_HSS0_RTS_N, 1);
+	gpio_set_value(port ? GPIO_HSS1_RTS_N : GPIO_HSS0_RTS_N, 1);
 }

@@ -414,13 +415,11 @@ static void __init gmlr_init(void)
 	if (hw_bits & CFG_HW_HAS_EEPROM)
 		device_tab[devices++] = &device_i2c; /* max index 6 */
 
-	gpio_line_config(GPIO_SCL, IXP4XX_GPIO_OUT);
-	gpio_line_config(GPIO_SDA, IXP4XX_GPIO_OUT);
-	gpio_line_config(GPIO_STR, IXP4XX_GPIO_OUT);
-	gpio_line_config(GPIO_HSS0_RTS_N, IXP4XX_GPIO_OUT);
-	gpio_line_config(GPIO_HSS1_RTS_N, IXP4XX_GPIO_OUT);
-	gpio_line_config(GPIO_HSS0_DCD_N, IXP4XX_GPIO_IN);
-	gpio_line_config(GPIO_HSS1_DCD_N, IXP4XX_GPIO_IN);
+	gpio_direction_output(GPIO_SCL, 1);
+	gpio_direction_output(GPIO_SDA, 1);
+	gpio_direction_output(GPIO_STR, 1);
+	gpio_direction_output(GPIO_HSS0_RTS_N, 0);
+	gpio_direction_output(GPIO_HSS1_RTS_N, 0);
 	irq_set_irq_type(IXP4XX_GPIO_IRQ(GPIO_HSS0_DCD_N), IRQ_TYPE_EDGE_BOTH);
 	irq_set_irq_type(IXP4XX_GPIO_IRQ(GPIO_HSS1_DCD_N), IRQ_TYPE_EDGE_BOTH);

arch/arm/mach-ixp4xx/include/mach/platform.h +0 ~~-38~~

index 2d3e3f1..7fef868 100644
@@ -132,47 +132,9 @@ struct pci_sys_data;
 extern int ixp4xx_setup(int nr, struct pci_sys_data *sys);
 extern struct pci_ops ixp4xx_ops;
 
-/*
- * GPIO-functions
- */
-/*
- * The following converted to the real HW bits the gpio_line_config
- */
-/* GPIO pin types */
-#define IXP4XX_GPIO_OUT 		0x1
-#define IXP4XX_GPIO_IN  		0x2
-
-/* GPIO signal types */
-#define IXP4XX_GPIO_LOW			0
-#define IXP4XX_GPIO_HIGH		1
-
 /* GPIO Clocks */
 #define IXP4XX_GPIO_CLK_0		14
 #define IXP4XX_GPIO_CLK_1		15
 
-/* using macros here to work around circular dependency with asm/io.h */
-#define gpio_line_config(line, direction)	\
-do {						\
-	u8 __line = (line);			\
-	if ((direction) == IXP4XX_GPIO_IN)	\
-		writel_relaxed(readl_relaxed(IXP4XX_GPIO_GPOER) | (1 << __line), IXP4XX_GPIO_GPOER); \
-	else					\
-		writel_relaxed(readl_relaxed(IXP4XX_GPIO_GPOER) & ~(1 << __line), IXP4XX_GPIO_GPOER); \
-} while (0)
-
-#define gpio_line_get(line, value)	\
-do {					\
-	*(value) = (readl_relaxed(IXP4XX_GPIO_GPINR) >> (line)) & 0x1; \
-} while(0)
-
-#define gpio_line_set(line, value)		\
-do {						\
-	u8 __line = (line);			\
-	if ((value) == IXP4XX_GPIO_HIGH)	\
-		writel_relaxed(readl_relaxed(IXP4XX_GPIO_GPOUTR) | (1 << __line), IXP4XX_GPIO_GPOUTR); \
-	else if ((value) == IXP4XX_GPIO_LOW)	\
-		writel_relaxed(readl_relaxed(IXP4XX_GPIO_GPOUTR) & ~(1 << __line), IXP4XX_GPIO_GPOUTR); \
-} while (0)
-
 #endif // __ASSEMBLY__

arch/arm/mach-ixp4xx/ixdp425-setup.c +3 -3

index 399c5e3..736900a 100644
@@ -80,10 +80,10 @@ ixdp425_flash_nand_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 
 	if (ctrl & NAND_CTRL_CHANGE) {
 		if (ctrl & NAND_NCE) {
-			gpio_line_set(IXDP425_NAND_NCE_PIN, IXP4XX_GPIO_LOW);
+			gpio_set_value(IXDP425_NAND_NCE_PIN, 0);
 			udelay(5);
 		} else
-			gpio_line_set(IXDP425_NAND_NCE_PIN, IXP4XX_GPIO_HIGH);
+			gpio_set_value(IXDP425_NAND_NCE_PIN, 1);
 
 		offset = (ctrl & NAND_CLE) ? IXDP425_NAND_CMD_BYTE : 0;
 		offset |= (ctrl & NAND_ALE) ? IXDP425_NAND_ADDR_BYTE : 0;

@@ -227,7 +227,7 @@ static void __init ixdp425_init(void)
 	ixdp425_flash_nand_resource.start = IXP4XX_EXP_BUS_BASE(3),
 	ixdp425_flash_nand_resource.end   = IXP4XX_EXP_BUS_BASE(3) + 0x10 - 1;
 
-	gpio_line_config(IXDP425_NAND_NCE_PIN, IXP4XX_GPIO_OUT);
+	gpio_direction_output(IXDP425_NAND_NCE_PIN, 1);
 
 	/* Configure expansion bus for NAND Flash */
 	*IXP4XX_EXP_CS3 = IXP4XX_EXP_BUS_CS_EN |

arch/arm/mach-ixp4xx/nas100d-setup.c +3 -6

index 647d377..373cbd2 100644
@@ -185,10 +185,7 @@ static void nas100d_power_off(void)
 	/* This causes the box to drop the power and go dead. */
 
 	/* enable the pwr cntl gpio */
-	gpio_line_config(NAS100D_PO_GPIO, IXP4XX_GPIO_OUT);
-
-	/* do the deed */
-	gpio_line_set(NAS100D_PO_GPIO, IXP4XX_GPIO_HIGH);
+	gpio_direction_output(NAS100D_PO_GPIO, 1);
 }
 
 /* This is used to make sure the power-button pusher is serious.  The button

@@ -225,7 +222,7 @@ static void nas100d_power_handler(unsigned long data)
 			ctrl_alt_del();
 
 			/* Change the state of the power LED to "blink" */
-			gpio_line_set(NAS100D_LED_PWR_GPIO, IXP4XX_GPIO_LOW);
+			gpio_set_value(NAS100D_LED_PWR_GPIO, 0);
 		} else {
 			power_button_countdown = PBUTTON_HOLDDOWN_COUNT;
 		}

@@ -284,7 +281,7 @@ static void __init nas100d_init(void)
 	 */
 
 	/* Make sure that the power button GPIO is set up as an input */
-	gpio_line_config(NAS100D_PB_GPIO, IXP4XX_GPIO_IN);
+	gpio_direction_input(NAS100D_PB_GPIO);
 
 	/* Set the initial value for the power button IRQ handler */
 	power_button_countdown = PBUTTON_HOLDDOWN_COUNT;

arch/arm/mach-ixp4xx/nslu2-setup.c +1 -4

index ada1175..6272e16 100644
@@ -198,10 +198,7 @@ static void nslu2_power_off(void)
 	/* This causes the box to drop the power and go dead. */
 
 	/* enable the pwr cntl gpio */
-	gpio_line_config(NSLU2_PO_GPIO, IXP4XX_GPIO_OUT);
-
-	/* do the deed */
-	gpio_line_set(NSLU2_PO_GPIO, IXP4XX_GPIO_HIGH);
+	gpio_direction_output(NSLU2_PO_GPIO, 1);
 }
 
 static irqreturn_t nslu2_power_handler(int irq, void *dev_id)

drivers/input/misc/ixp4xx-beeper.c +26 ~~-23~~

130 lines not on this page · raw message

drivers/ptp/ptp_ixp46x.c +21 ~~-10~~

index d49b851..84ac0d2 100644
@@ -260,8 +260,6 @@ static int setup_interrupt(int gpio)
 {
 	int irq;
 
-	gpio_line_config(gpio, IXP4XX_GPIO_IN);
-
 	irq = gpio_to_irq(gpio);
 
 	if (NO_IRQ == irq)

@@ -280,14 +278,6 @@ static int setup_interrupt(int gpio)
 	return irq;
 }
 
-static void __exit ptp_ixp_exit(void)
-{
-	free_irq(MASTER_IRQ, &ixp_clock);
-	free_irq(SLAVE_IRQ, &ixp_clock);
-	ixp46x_phc_index = -1;
-	ptp_clock_unregister(ixp_clock.ptp_clock);
-}
-
 static int __init ptp_ixp_init(void)
 {
 	if (!cpu_is_ixp46x())

@@ -310,6 +300,9 @@ static int __init ptp_ixp_init(void)
 	__raw_writel(0, &ixp_clock.regs->trgt_hi);
 	__raw_writel(TTIPEND, &ixp_clock.regs->event);
 
+	gpio_request_one(MASTER_GPIO, GPIOF_IN, "PTP Master");
+	gpio_request_one(SLAVE_GPIO, GPIOF_IN, "PTP Slave");
+
 	if (MASTER_IRQ != setup_interrupt(MASTER_GPIO)) {
 		pr_err("failed to setup gpio %d as irq\n", MASTER_GPIO);
 		goto no_master;

@@ -320,13 +313,31 @@ static int __init ptp_ixp_init(void)
 	}
 
 	return 0;
+
 no_slave:
 	free_irq(MASTER_IRQ, &ixp_clock);
+
 no_master:
+	gpio_free(MASTER_GPIO);
+	gpio_free(SLAVE_GPIO);
+
 	ptp_clock_unregister(ixp_clock.ptp_clock);
+
 	return -ENODEV;
 }
 
+static void __exit ptp_ixp_exit(void)
+{
+	free_irq(MASTER_IRQ, &ixp_clock);
+	free_irq(SLAVE_IRQ, &ixp_clock);
+
+	gpio_free(MASTER_GPIO);
+	gpio_free(SLAVE_GPIO);
+
+	ixp46x_phc_index = -1;
+	ptp_clock_unregister(ixp_clock.ptp_clock);
+}
+
 module_init(ptp_ixp_init);
 module_exit(ptp_ixp_exit);

Signature · 1 line

1.7.8.6

Arnd Bergmann

2012-12-02 13:25 UTC #13 lore raw

PATCH 2/2 ARM: ixp4xx: Using gpiolib rather than a private GPIO API

Quoted 13 lines · Alexander Shiyan

Signed-off-by Alexander Shiyan <[email protected]>

---
 arch/arm/mach-ixp4xx/common.c                |   29 ++++++++++-----
 arch/arm/mach-ixp4xx/dsmg600-setup.c         |    9 ++---
 arch/arm/mach-ixp4xx/goramo_mlr.c            |   31 ++++++++--------
 arch/arm/mach-ixp4xx/include/mach/platform.h |   38 --------------------
 arch/arm/mach-ixp4xx/ixdp425-setup.c         |    6 ++--
 arch/arm/mach-ixp4xx/nas100d-setup.c         |    9 ++---
 arch/arm/mach-ixp4xx/nslu2-setup.c           |    5 +--
 drivers/input/misc/ixp4xx-beeper.c           |   49 ++++++++++++++------------
 drivers/ptp/ptp_ixp46x.c                     |   31 +++++++++++-----
 9 files changed, 91 insertions(+), 116 deletions(-)

Very nice!

Acked-by Arnd Bergmann <[email protected]>

Arnd Bergmann

2012-12-02 13:24 UTC #14 lore raw

PATCH 1/2 ARM: ixp4xx: Moving the timer flags control in ixp4xx_irq_ack() procedure

In reply to Alexander Shiyan

Quoted 5 lines · Alexander Shiyan

Rather than modify the EOI flags directly in the timer interrupt, let's deal with these flags in the ixp4xx_irq_ack() procedure.
Signed-off-by Alexander Shiyan <[email protected]>

Acked-by Arnd Bergmann <[email protected]>

Mikael Pettersson

2012-12-02 13:37 UTC #15 lore raw

PATCH 1/2 ARM: ixp4xx: Moving the timer flags control in ixp4xx_irq_ack() procedure

In reply to Alexander Shiyan

Quoted 2 lines · Alexander Shiyan

Rather than modify the EOI flags directly in the timer interrupt, let's deal with these flags in the ixp4xx_irq_ack() procedure.

Why?

All I see is that you're removing simple unconditional code from two interrupt handlers and adding a large conditional to ixp4xx_irq_ack, which will slow down all interrupt acks.

So in what way is this an improvement?

/Mikael

Quoted 98 lines

Signed-off-by Alexander Shiyan <[email protected]>

---
 arch/arm/mach-ixp4xx/common.c      |   35 ++++++++++++++++++++++++++---------
 drivers/input/misc/ixp4xx-beeper.c |    3 ---
 2 files changed, 26 insertions(+), 12 deletions(-)

arch/arm/mach-ixp4xx/common.c +26 -9

index 7443e1d..8e2a3b8 100644
@@ -204,10 +204,28 @@ static void ixp4xx_irq_mask(struct irq_data *d)
 
 static void ixp4xx_irq_ack(struct irq_data *d)
 {
-	int line = (d->irq < 32) ? irq2gpio[d->irq] : -1;
+	int line;
 
-	if (line >= 0)
-		writel_relaxed(1 << line, IXP4XX_GPIO_GPISR);
+	switch (d->irq) {
+	case IRQ_IXP4XX_TIMER1:
+		writel_relaxed(IXP4XX_OSST_TIMER_1_PEND, IXP4XX_OSST);
+		break;
+	case IRQ_IXP4XX_TIMER2:
+		writel_relaxed(IXP4XX_OSST_TIMER_2_PEND, IXP4XX_OSST);
+		break;
+	case IRQ_IXP4XX_TIMESTAMP:
+		writel_relaxed(IXP4XX_OSST_TIMER_TS_PEND, IXP4XX_OSST);
+		break;
+	case IRQ_IXP4XX_WDOG:
+		writel_relaxed(IXP4XX_OSST_TIMER_WDOG_PEND, IXP4XX_OSST);
+		break;
+	default:
+		line = (d->irq < 32) ? irq2gpio[d->irq] : -1;
+
+		if (line >= 0)
+			writel_relaxed(1 << line, IXP4XX_GPIO_GPISR);
+		break;
+	}
 }
 
 /*

@@ -249,6 +267,11 @@ void __init ixp4xx_init_irq(void)
 	/* Disable all interrupt */
 	writel_relaxed(0, IXP4XX_ICMR);
 
+	/* Clear Timer Pending Interrupts */
+	writel_relaxed(IXP4XX_OSST_TIMER_1_PEND | IXP4XX_OSST_TIMER_2_PEND |
+		       IXP4XX_OSST_TIMER_TS_PEND | IXP4XX_OSST_TIMER_WDOG_PEND,
+		       IXP4XX_OSST);
+
 	if (cpu_is_ixp46x() || cpu_is_ixp43x()) {
 		/* Route upper 32 sources to IRQ instead of FIQ */
 		writel_relaxed(0, IXP4XX_ICLR2);

@@ -276,9 +299,6 @@ static irqreturn_t ixp4xx_timer_interrupt(int irq, void *dev_id)
 {
 	struct clock_event_device *evt = dev_id;
 
-	/* Clear Pending Interrupt by writing '1' to it */
-	writel_relaxed(IXP4XX_OSST_TIMER_1_PEND, IXP4XX_OSST);
-
 	evt->event_handler(evt);
 
 	return IRQ_HANDLED;

@@ -296,9 +316,6 @@ void __init ixp4xx_timer_init(void)
 	/* Reset/disable counter */
 	writel_relaxed(0, IXP4XX_OSRT1);
 
-	/* Clear Pending Interrupt by writing '1' to it */
-	writel_relaxed(IXP4XX_OSST_TIMER_1_PEND, IXP4XX_OSST);
-
 	/* Reset time-stamp counter */
 	writel_relaxed(0, IXP4XX_OSTS);

drivers/input/misc/ixp4xx-beeper.c +0 -3

index eae50fe..6b40950 100644
@@ -78,9 +78,6 @@ static int ixp4xx_spkr_event(struct input_dev *dev, unsigned int type, unsigned
 
 static irqreturn_t ixp4xx_spkr_interrupt(int irq, void *dev_id)
 {
-	/* clear interrupt */
-	writel_relaxed(IXP4XX_OSST_TIMER_2_PEND, IXP4XX_OSST);
-
 	/* flip the beeper output */
 	writel_relaxed(readl_relaxed(IXP4XX_GPIO_GPOUTR) ^ (1 << (unsigned int) dev_id), IXP4XX_GPIO_GPOUTR);

Signature · 7 lines

1.7.8.6


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

Alexander Shiyan

author

2012-12-02 13:50 UTC #16 lore raw

Re[2]: [PATCH 1/2] ARM: ixp4xx: Moving the timer flags control in ixp4xx_irq_ack() procedure

Quoted 11 lines

Quoted 2 lines · Alexander Shiyan
Rather than modify the EOI flags directly in the timer interrupt, let's deal with these flags in the ixp4xx_irq_ack() procedure.
Why?
All I see is that you're removing simple unconditional code from two interrupt handlers and adding a large conditional to ixp4xx_irq_ack, which will slow down all interrupt acks.
So in what way is this an improvement?

This is a right place for do it. Now we can use these interrupts in any drivers without patching last one for touching ACK.

---