Skip to contentSkip to conversation

rb532: use physical addresses for gpio and device controller registers

PATCH In review Florian Fainelli 2 participants 3 messages Aug 21, 2008 – Aug 22, 2008 MIPS

lore
Share & tools
lore
#0 lore raw

PATCH 2/6 rb532: use physical addresses for gpio and device controller registers

This patch fixes the misuse of virtual address which would lead to serious problems while accessing ioremap'd registers in the GPIO code.

---

arch/mips/rb532/gpio.c +4 -4
index 00a1c78..3d1632c 100644
@@ -47,8 +47,8 @@ struct mpmc_device dev3;
 static struct resource rb532_gpio_reg0_res[] = {
 	{
 		.name 	= "gpio_reg0",
-		.start 	= (u32)(IDT434_REG_BASE + GPIOBASE),
-		.end 	= (u32)(IDT434_REG_BASE + GPIOBASE + sizeof(struct rb532_gpio_reg)),
+		.start 	= (REG_BASE + GPIOBASE),
+		.end 	= (REG_BASE + GPIOBASE + sizeof(struct rb532_gpio_reg)),
 		.flags 	= IORESOURCE_MEM,
 	}
 };
@@ -56,8 +56,8 @@ static struct resource rb532_gpio_reg0_res[] = {
 static struct resource rb532_dev3_ctl_res[] = {
 	{
 		.name	= "dev3_ctl",
-		.start	= (u32)(IDT434_REG_BASE + DEV3BASE),
-		.end	= (u32)(IDT434_REG_BASE + DEV3BASE + sizeof(struct dev_reg)),
+		.start	= (REG_BASE + DEV3BASE),
+		.end	= (REG_BASE + DEV3BASE + sizeof(struct dev_reg)),
 		.flags	= IORESOURCE_MEM,
 	}
 };
include/asm-mips/mach-rc32434/rb.h +2 -0
index e0a76e3..f229da4 100644
@@ -18,6 +18,7 @@
 #include <linux/genhd.h>
 
 #define IDT434_REG_BASE	((volatile void *) KSEG1ADDR(0x18000000))
+#define REG_BASE	0x18000000
 #define DEV0BASE	0x010000
 #define DEV0MASK	0x010004
 #define DEV0C		0x010008
@@ -43,6 +44,7 @@
 #define GPIOISTAT	0x050010
 #define GPIONMIEN	0x050014
 #define IMASK6		0x038038
+
 #define LO_WPX		(1 << 0)
 #define LO_ALE		(1 << 1)
 #define LO_CLE		(1 << 2)
#1 lore raw

Hello.

Quoted 23 lines · Florian Fainelli

This patch fixes the misuse of virtual address which would lead to serious problems while accessing ioremap'd registers in the GPIO code.

---

arch/mips/rb532/gpio.c +2 -2
index 00a1c78..3d1632c 100644
@@ -47,8 +47,8 @@ struct mpmc_device dev3;
 static struct resource rb532_gpio_reg0_res[] = {
 	{
 		.name 	= "gpio_reg0",
-		.start 	= (u32)(IDT434_REG_BASE + GPIOBASE),
-		.end 	= (u32)(IDT434_REG_BASE + GPIOBASE + sizeof(struct rb532_gpio_reg)),
+		.start 	= (REG_BASE + GPIOBASE),
+		.end 	= (REG_BASE + GPIOBASE + sizeof(struct rb532_gpio_reg)),
 		.flags 	= IORESOURCE_MEM,
 	}
 };

arch/mips/rb532/gpio.c static struct resource rb532_gpio_reg0_res[] = { · line 63

Quoted 10 lines
arch/mips/rb532/gpio.c +2 -2
@@ -56,8 +56,8 @@ static struct resource rb532_gpio_reg0_res[] = {
 static struct resource rb532_dev3_ctl_res[] = {
 	{
 		.name	= "dev3_ctl",
-		.start	= (u32)(IDT434_REG_BASE + DEV3BASE),
-		.end	= (u32)(IDT434_REG_BASE + DEV3BASE + sizeof(struct dev_reg)),
+		.start	= (REG_BASE + DEV3BASE),
+		.end	= (REG_BASE + DEV3BASE + sizeof(struct dev_reg)),
 		.flags	= IORESOURCE_MEM,
 	}
 };

I think .end initializers are wrong in both cases, you should've
subtracted one while at it.
You could aso have used CPHYSADDR(IDT434_REG_BASE) ISO defining
REG_BASE...

include/asm-mips/mach-rc32434/rb.h line 21

Quoted 9 lines
include/asm-mips/mach-rc32434/rb.h +1 -0
index e0a76e3..f229da4 100644
@@ -18,6 +18,7 @@
 #include <linux/genhd.h>
 
 #define IDT434_REG_BASE	((volatile void *) KSEG1ADDR(0x18000000))
+#define REG_BASE	0x18000000

I think you'd better #define IDT434_REG_BASE via REG_BASE.

WBR, Sergei

#2 lore raw
In reply to Florian Fainelli
Quoted 22 lines · Florian Fainelli

This patch fixes the misuse of virtual address which would lead to serious problems while accessing ioremap'd registers in the GPIO code.

---

arch/mips/rb532/gpio.c +2 -2
index 00a1c78..3d1632c 100644
@@ -47,8 +47,8 @@ struct mpmc_device dev3;
 static struct resource rb532_gpio_reg0_res[] = {
 	{
 		.name 	= "gpio_reg0",
-		.start 	= (u32)(IDT434_REG_BASE + GPIOBASE),
-		.end 	= (u32)(IDT434_REG_BASE + GPIOBASE + sizeof(struct rb532_gpio_reg)),
+		.start 	= (REG_BASE + GPIOBASE),
+		.end 	= (REG_BASE + GPIOBASE + sizeof(struct rb532_gpio_reg)),
 		.flags 	= IORESOURCE_MEM,
 	}
 };

arch/mips/rb532/gpio.c static struct resource rb532_gpio_reg0_res[] = { · line 63

Quoted 10 lines
arch/mips/rb532/gpio.c +2 -2
@@ -56,8 +56,8 @@ static struct resource rb532_gpio_reg0_res[] = {
 static struct resource rb532_dev3_ctl_res[] = {
 	{
 		.name	= "dev3_ctl",
-		.start	= (u32)(IDT434_REG_BASE + DEV3BASE),
-		.end	= (u32)(IDT434_REG_BASE + DEV3BASE + sizeof(struct dev_reg)),
+		.start	= (REG_BASE + DEV3BASE),
+		.end	= (REG_BASE + DEV3BASE + sizeof(struct dev_reg)),
 		.flags	= IORESOURCE_MEM,
 	}
 };

Oh, and parens are not needed in the initializers anymore...

WBR, Sergei