Skip to contentSkip to conversation

balance ioremap/iounmap in {sycamore, walnut}_setup_arch()

PATCH In review Roel Kluin 2 participants 3 messages Nov 8, 2007 – Nov 9, 2007

lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 balance ioremap/iounmap in {sycamore, walnut}_setup_arch()

I guess it should be done after the last usage of kb_data or fpga_status? --
Balance ioremap/iounmap

Signed-off-by Roel Kluin <[email protected]>

---

arch/ppc/platforms/4xx/sycamore.c +2 -0
index 8689f3e..4f3bac1 100644
@@ -99,28 +99,30 @@ sycamore_setup_arch(void)
 	kb_data = ioremap(SYCAMORE_PS2_BASE, 8);
 	if (!kb_data) {
 		printk(KERN_CRIT
 		       "sycamore_setup_arch() kb_data ioremap failed\n");
 		return;
 	}
 
 	kb_cs = kb_data + 1;
+	iounmap(kb_data);
 
 	fpga_status = ioremap(PPC40x_FPGA_BASE, 8);
 	if (!fpga_status) {
 		printk(KERN_CRIT
 		       "sycamore_setup_arch() fpga_status ioremap failed\n");
 		return;
 	}
 
 	fpga_enable = fpga_status + 1;
 	fpga_polarity = fpga_status + 2;
 	fpga_trigger = fpga_status + 3;
 	fpga_brdc = fpga_status + 4;
+	iounmap(fpga_status);
 
 	/* split the keyboard and mouse interrupts */
 	fpga_brdc_data = readb(fpga_brdc);
 	fpga_brdc_data |= 0x80;
 	writeb(fpga_brdc_data, fpga_brdc);
 
 	writeb(0x3, fpga_enable);
arch/ppc/platforms/4xx/walnut.c +2 -0
index 2f97723..8cfeb94 100644
@@ -81,28 +81,30 @@ walnut_setup_arch(void)
 	kb_data = ioremap(WALNUT_PS2_BASE, 8);
 	if (!kb_data) {
 		printk(KERN_CRIT
 		       "walnut_setup_arch() kb_data ioremap failed\n");
 		return;
 	}
 
 	kb_cs = kb_data + 1;
+	iounmap(kb_data);
 
 	fpga_status = ioremap(PPC40x_FPGA_BASE, 8);
 	if (!fpga_status) {
 		printk(KERN_CRIT
 		       "walnut_setup_arch() fpga_status ioremap failed\n");
 		return;
 	}
 
 	fpga_enable = fpga_status + 1;
 	fpga_polarity = fpga_status + 2;
 	fpga_trigger = fpga_status + 3;
 	fpga_brdc = fpga_status + 4;
+	iounmap(fpga_status);
 
 	/* split the keyboard and mouse interrupts */
 	fpga_brdc_data = readb(fpga_brdc);
 	fpga_brdc_data |= 0x80;
 	writeb(fpga_brdc_data, fpga_brdc);
 
 	writeb(0x3, fpga_enable);
#1 lore raw
Quoted 1 line · Roel Kluin

I guess it should be done after the last usage of kb_data or fpga_status?

I think no iounmap(kb_data) needed. Looks like the pointer kn_cs (kb_cs = kb_data + 1) is used by the serio driver (drivers/input/serio/i8042-ppcio.h). Please note that we just ioremap and assign pointers here, not actually reading the registers.
Also, looks like you unmap the fpga registers too early.
Thanks,
Valentine.

Quoted 79 lines

--
Balance ioremap/iounmap

Signed-off-by Roel Kluin <[email protected]>

---

arch/ppc/platforms/4xx/sycamore.c +2 -0
index 8689f3e..4f3bac1 100644
@@ -99,28 +99,30 @@ sycamore_setup_arch(void)
 	kb_data = ioremap(SYCAMORE_PS2_BASE, 8);
 	if (!kb_data) {
 		printk(KERN_CRIT
 		       "sycamore_setup_arch() kb_data ioremap failed\n");
 		return;
 	}
 
 	kb_cs = kb_data + 1;
+	iounmap(kb_data);
 
 	fpga_status = ioremap(PPC40x_FPGA_BASE, 8);
 	if (!fpga_status) {
 		printk(KERN_CRIT
 		       "sycamore_setup_arch() fpga_status ioremap failed\n");
 		return;
 	}
 
 	fpga_enable = fpga_status + 1;
 	fpga_polarity = fpga_status + 2;
 	fpga_trigger = fpga_status + 3;
 	fpga_brdc = fpga_status + 4;
+	iounmap(fpga_status);
 
 	/* split the keyboard and mouse interrupts */
 	fpga_brdc_data = readb(fpga_brdc);
 	fpga_brdc_data |= 0x80;
 	writeb(fpga_brdc_data, fpga_brdc);
 
 	writeb(0x3, fpga_enable);
arch/ppc/platforms/4xx/walnut.c +2 -0
index 2f97723..8cfeb94 100644
@@ -81,28 +81,30 @@ walnut_setup_arch(void)
 	kb_data = ioremap(WALNUT_PS2_BASE, 8);
 	if (!kb_data) {
 		printk(KERN_CRIT
 		       "walnut_setup_arch() kb_data ioremap failed\n");
 		return;
 	}
 
 	kb_cs = kb_data + 1;
+	iounmap(kb_data);
 
 	fpga_status = ioremap(PPC40x_FPGA_BASE, 8);
 	if (!fpga_status) {
 		printk(KERN_CRIT
 		       "walnut_setup_arch() fpga_status ioremap failed\n");
 		return;
 	}
 
 	fpga_enable = fpga_status + 1;
 	fpga_polarity = fpga_status + 2;
 	fpga_trigger = fpga_status + 3;
 	fpga_brdc = fpga_status + 4;
+	iounmap(fpga_status);
 
 	/* split the keyboard and mouse interrupts */
 	fpga_brdc_data = readb(fpga_brdc);
 	fpga_brdc_data |= 0x80;
 	writeb(fpga_brdc_data, fpga_brdc);
 
 	writeb(0x3, fpga_enable);

_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-dev

author
#2 lore raw
Quoted 11 lines · Valentine Barshak
Quoted 1 line · Roel Kluin

I guess it should be done after the last usage of kb_data or fpga_status?

I think no iounmap(kb_data) needed. Looks like the pointer kn_cs (kb_cs = kb_data + 1) is used by the serio driver (drivers/input/serio/i8042-ppcio.h).
Please note that we just ioremap and assign pointers here, not actually reading the registers.
Also, looks like you unmap the fpga registers too early. Thanks,
Valentine.

Thanks for your pointers. I'm new, so I'm not sure how this should be done. I have removed the iounmap(kb_data), and moved the iounmap(fpga_status) lower. possibly the latter could be done before the sycamore_rtc_base assignment? --
Balance ioremap/iounmap

Signed-off-by Roel Kluin <[email protected]>

---

arch/ppc/platforms/4xx/sycamore.c +1 -0
index 8689f3e..6ce4815 100644
@@ -132,6 +132,7 @@ sycamore_setup_arch(void)
 	sycamore_rtc_base = (void *) SYCAMORE_RTC_VADDR;
 	TODC_INIT(TODC_TYPE_DS1743, sycamore_rtc_base, sycamore_rtc_base,
 		  sycamore_rtc_base, 8);
+	iounmap(fpga_status);
 
 	/* Identify the system */
 	printk(KERN_INFO "IBM Sycamore (IBM405GPr) Platform\n");