Skip to contentSkip to conversation

check request_region() return value in arch/ppc and

PATCH In review Tim Cooijmans 3 participants 7 messages Feb 27, 2006 – Mar 1, 2006

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 check request_region() return value in arch/ppc and

From: Tim Cooijmans <[email protected]>

Check request_region() return value and warn on failure. Note that releasing succeeded requests doesn't make much sense, as the calling code doesn't know if anything failed.

The powerpc code has been compile tested. The ppc code has not been tested, but the changes are very similar.

---

arch/powerpc/platforms/chrp/setup.c +14 -6
diff -uprN linux-2.6.16-rc5-orig/arch/powerpc/platforms/chrp/setup.c linux-2.6.16-rc5/arch/powerpc/platforms/chrp/setup.c
@@ -465,18 +465,26 @@ void __init chrp_init_IRQ(void)
 }
 
 void __init
+chrp_request_region(unsigned long start, unsigned long n, char *desc)
+{
+	if (!request_region(start, n, desc)) {
+		printk(KERN_WARNING "CHRP: unable to allocate %s region.\n", desc);
+	}
+}
+
+void __init
 chrp_init2(void)
 {
 #ifdef CONFIG_NVRAM
 	chrp_nvram_init();
 #endif
 
-	request_region(0x20,0x20,"pic1");
-	request_region(0xa0,0x20,"pic2");
-	request_region(0x00,0x20,"dma1");
-	request_region(0x40,0x20,"timer");
-	request_region(0x80,0x10,"dma page reg");
-	request_region(0xc0,0x20,"dma2");
+	chrp_request_region(0x20,0x20,"pic1");
+	chrp_request_region(0xa0,0x20,"pic2");
+	chrp_request_region(0x00,0x20,"dma1");
+	chrp_request_region(0x40,0x20,"timer");
+	chrp_request_region(0x80,0x10,"dma page reg");
+	chrp_request_region(0xc0,0x20,"dma2");
 
 	if (ppc_md.progress)
 		ppc_md.progress("  Have fun!    ", 0x7777);
arch/powerpc/platforms/pseries/pci.c +14 -6
diff -uprN linux-2.6.16-rc5-orig/arch/powerpc/platforms/pseries/pci.c linux-2.6.16-rc5/arch/powerpc/platforms/pseries/pci.c
@@ -92,17 +92,25 @@ void __devinit pSeries_irq_bus_setup(str
 	}
 }
 
+void __init
+pSeries_request_region(unsigned long start, unsigned long n, char *desc)
+{
+	if (!request_region(start, n, desc)) {
+		printk(KERN_WARNING "pSeries: unable to allocate %s region.\n", desc);
+	}
+}
+
 static void __init pSeries_request_regions(void)
 {
 	if (!isa_io_base)
 		return;
 
-	request_region(0x20,0x20,"pic1");
-	request_region(0xa0,0x20,"pic2");
-	request_region(0x00,0x20,"dma1");
-	request_region(0x40,0x20,"timer");
-	request_region(0x80,0x10,"dma page reg");
-	request_region(0xc0,0x20,"dma2");
+	pSeries_request_region(0x20,0x20,"pic1");
+	pSeries_request_region(0xa0,0x20,"pic2");
+	pSeries_request_region(0x00,0x20,"dma1");
+	pSeries_request_region(0x40,0x20,"timer");
+	pSeries_request_region(0x80,0x10,"dma page reg");
+	pSeries_request_region(0xc0,0x20,"dma2");
 }
 
 void __init pSeries_final_fixup(void)
arch/ppc/platforms/chrp_setup.c +14 -6
diff -uprN linux-2.6.16-rc5-orig/arch/ppc/platforms/chrp_setup.c linux-2.6.16-rc5/arch/ppc/platforms/chrp_setup.c
@@ -450,18 +450,26 @@ void __init chrp_init_IRQ(void)
 }
 
 void __init
+chrp_request_region(unsigned long start, unsigned long n, char *desc)
+{
+	if (!request_region(start, n, desc)) {
+		printk(KERN_WARNING "CHRP: unable to allocate %s region.\n", desc);
+	}
+}
+
+void __init
 chrp_init2(void)
 {
 #ifdef CONFIG_NVRAM
 	chrp_nvram_init();
 #endif
 
-	request_region(0x20,0x20,"pic1");
-	request_region(0xa0,0x20,"pic2");
-	request_region(0x00,0x20,"dma1");
-	request_region(0x40,0x20,"timer");
-	request_region(0x80,0x10,"dma page reg");
-	request_region(0xc0,0x20,"dma2");
+	chrp_request_region(0x20,0x20,"pic1");
+	chrp_request_region(0xa0,0x20,"pic2");
+	chrp_request_region(0x00,0x20,"dma1");
+	chrp_request_region(0x40,0x20,"timer");
+	chrp_request_region(0x80,0x10,"dma page reg");
+	chrp_request_region(0xc0,0x20,"dma2");
 
 	if (ppc_md.progress)
 		ppc_md.progress("  Have fun!    ", 0x7777);
arch/ppc/platforms/mvme5100.c +14 -6
diff -uprN linux-2.6.16-rc5-orig/arch/ppc/platforms/mvme5100.c linux-2.6.16-rc5/arch/ppc/platforms/mvme5100.c
@@ -189,16 +189,24 @@ mvme5100_setup_arch(void)
 	return;
 }
 
+void __init
+mvme5100_request_region(unsigned long start, unsigned long n, char *desc)
+{
+	if (!request_region(start, n, desc)) {
+		printk(KERN_WARNING "mvme5100: unable to allocate %s region.\n", desc);
+	}
+}
+
 static void __init
 mvme5100_init2(void)
 {
 #ifdef CONFIG_MVME5100_IPMC761_PRESENT
-		request_region(0x00,0x20,"dma1");
-		request_region(0x20,0x20,"pic1");
-		request_region(0x40,0x20,"timer");
-		request_region(0x80,0x10,"dma page reg");
-		request_region(0xa0,0x20,"pic2");
-		request_region(0xc0,0x20,"dma2");
+		mvme5100_request_region(0x00,0x20,"dma1");
+		mvme5100_request_region(0x20,0x20,"pic1");
+		mvme5100_request_region(0x40,0x20,"timer");
+		mvme5100_request_region(0x80,0x10,"dma page reg");
+		mvme5100_request_region(0xa0,0x20,"pic2");
+		mvme5100_request_region(0xc0,0x20,"dma2");
 #endif
 	return;
 }
arch/ppc/platforms/pplus.c +14 -6
diff -uprN linux-2.6.16-rc5-orig/arch/ppc/platforms/pplus.c linux-2.6.16-rc5/arch/ppc/platforms/pplus.c
@@ -814,17 +814,25 @@ static void __init pplus_map_io(void)
 	io_block_mapping(0xfef80000, 0xfef80000, 0x00080000, _PAGE_IO);
 }
 
+void __init
+pplus_request_region(unsigned long start, unsigned long n, char *desc)
+{
+	if (!request_region(start, n, desc)) {
+		printk(KERN_WARNING "pplus: unable to allocate %s region.\n", desc);
+	}
+}
+
 static void __init pplus_init2(void)
 {
 #ifdef CONFIG_NVRAM
 	request_region(PREP_NVRAM_AS0, 0x8, "nvram");
 #endif
-	request_region(0x20, 0x20, "pic1");
-	request_region(0xa0, 0x20, "pic2");
-	request_region(0x00, 0x20, "dma1");
-	request_region(0x40, 0x20, "timer");
-	request_region(0x80, 0x10, "dma page reg");
-	request_region(0xc0, 0x20, "dma2");
+	pplus_request_region(0x20, 0x20, "pic1");
+	pplus_request_region(0xa0, 0x20, "pic2");
+	pplus_request_region(0x00, 0x20, "dma1");
+	pplus_request_region(0x40, 0x20, "timer");
+	pplus_request_region(0x80, 0x10, "dma page reg");
+	pplus_request_region(0xc0, 0x20, "dma2");
 }
 
 /*

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#1 lore raw

PATCH 1/1 check request_region() return value in arch/ppc and

From: Tim Cooijmans <[email protected]>

Check request_region() return value and warn on failure. Note that releasing succeeded requests doesn't make much sense, as the calling code doesn't know if anything failed.

The powerpc code has been compile tested. The ppc code has not been tested, but the changes are very similar.

---
diff -uprN linux-2.6.16-rc5-orig/arch/powerpc/platforms/chrp/setup.c linux-2.6.16-rc5/arch/powerpc/platforms/chrp/setup.c
--- linux-2.6.16-rc5-orig/arch/powerpc/platforms/chrp/setup.c 2006-02-27
20:36:36.000000000 +0100
+++ linux-2.6.16-rc5/arch/powerpc/platforms/chrp/setup.c 2006-02-27
21:52:27.000000000 +0100
@@ -465,18 +465,25 @@ void __init chrp_init_IRQ(void)
}

void __init
+chrp_request_region(unsigned long start, unsigned long n, char *desc) +{
+ if (!request_region(start, n, desc))
+ printk(KERN_WARNING "CHRP: unable to allocate %s region.\n", desc);
+}
+
+void __init
chrp_init2(void)
{
#ifdef CONFIG_NVRAM
chrp_nvram_init();
#endif

- request_region(0x20,0x20,"pic1");
- request_region(0xa0,0x20,"pic2");
- request_region(0x00,0x20,"dma1");
- request_region(0x40,0x20,"timer");
- request_region(0x80,0x10,"dma page reg");
- request_region(0xc0,0x20,"dma2");
+ chrp_request_region(0x20,0x20,"pic1");
+ chrp_request_region(0xa0,0x20,"pic2");
+ chrp_request_region(0x00,0x20,"dma1");
+ chrp_request_region(0x40,0x20,"timer");
+ chrp_request_region(0x80,0x10,"dma page reg");
+ chrp_request_region(0xc0,0x20,"dma2");

if (ppc_md.progress)
ppc_md.progress(" Have fun! ", 0x7777);
diff -uprN linux-2.6.16-rc5-orig/arch/powerpc/platforms/pseries/pci.c linux-2.6.16-rc5/arch/powerpc/platforms/pseries/pci.c
--- linux-2.6.16-rc5-orig/arch/powerpc/platforms/pseries/pci.c 2006-02-27
20:36:36.000000000 +0100
+++ linux-2.6.16-rc5/arch/powerpc/platforms/pseries/pci.c 2006-02-27
21:49:54.000000000 +0100
@@ -92,17 +92,24 @@ void __devinit pSeries_irq_bus_setup(str
}
}

+void __init
+pSeries_request_region(unsigned long start, unsigned long n, char *desc) +{
+ if (!request_region(start, n, desc))
+ printk(KERN_WARNING "pSeries: unable to allocate %s region.\n", desc);
+}
+
static void __init pSeries_request_regions(void)
{
if (!isa_io_base)
return;

- request_region(0x20,0x20,"pic1");
- request_region(0xa0,0x20,"pic2");
- request_region(0x00,0x20,"dma1");
- request_region(0x40,0x20,"timer");
- request_region(0x80,0x10,"dma page reg");
- request_region(0xc0,0x20,"dma2");
+ pSeries_request_region(0x20,0x20,"pic1");
+ pSeries_request_region(0xa0,0x20,"pic2");
+ pSeries_request_region(0x00,0x20,"dma1");
+ pSeries_request_region(0x40,0x20,"timer");
+ pSeries_request_region(0x80,0x10,"dma page reg");
+ pSeries_request_region(0xc0,0x20,"dma2");
}

void __init pSeries_final_fixup(void)
diff -uprN linux-2.6.16-rc5-orig/arch/ppc/platforms/chrp_setup.c linux-2.6.16-rc5/arch/ppc/platforms/chrp_setup.c
--- linux-2.6.16-rc5-orig/arch/ppc/platforms/chrp_setup.c 2006-02-27
20:36:36.000000000 +0100
+++ linux-2.6.16-rc5/arch/ppc/platforms/chrp_setup.c 2006-02-27
21:52:29.000000000 +0100
@@ -450,18 +450,25 @@ void __init chrp_init_IRQ(void)
}

void __init
+chrp_request_region(unsigned long start, unsigned long n, char *desc) +{
+ if (!request_region(start, n, desc))
+ printk(KERN_WARNING "CHRP: unable to allocate %s region.\n", desc);
+}
+
+void __init
chrp_init2(void)
{
#ifdef CONFIG_NVRAM
chrp_nvram_init();
#endif

- request_region(0x20,0x20,"pic1");
- request_region(0xa0,0x20,"pic2");
- request_region(0x00,0x20,"dma1");
- request_region(0x40,0x20,"timer");
- request_region(0x80,0x10,"dma page reg");
- request_region(0xc0,0x20,"dma2");
+ chrp_request_region(0x20,0x20,"pic1");
+ chrp_request_region(0xa0,0x20,"pic2");
+ chrp_request_region(0x00,0x20,"dma1");
+ chrp_request_region(0x40,0x20,"timer");
+ chrp_request_region(0x80,0x10,"dma page reg");
+ chrp_request_region(0xc0,0x20,"dma2");

if (ppc_md.progress)
ppc_md.progress(" Have fun! ", 0x7777);
diff -uprN linux-2.6.16-rc5-orig/arch/ppc/platforms/mvme5100.c linux-2.6.16-rc5/arch/ppc/platforms/mvme5100.c
--- linux-2.6.16-rc5-orig/arch/ppc/platforms/mvme5100.c 2006-02-27
20:36:36.000000000 +0100
+++ linux-2.6.16-rc5/arch/ppc/platforms/mvme5100.c 2006-02-27
21:57:46.000000000 +0100
@@ -189,16 +189,23 @@ mvme5100_setup_arch(void)
return;
}

+void __init
+mvme5100_request_region(unsigned long start, unsigned long n, char *desc) +{
+ if (!request_region(start, n, desc))
+ printk(KERN_WARNING "mvme5100: unable to allocate %s region.\n", desc);
+}
+
static void __init
mvme5100_init2(void)
{
#ifdef CONFIG_MVME5100_IPMC761_PRESENT
- request_region(0x00,0x20,"dma1");
- request_region(0x20,0x20,"pic1");
- request_region(0x40,0x20,"timer");
- request_region(0x80,0x10,"dma page reg");
- request_region(0xa0,0x20,"pic2");
- request_region(0xc0,0x20,"dma2");
+ mvme5100_request_region(0x00,0x20,"dma1");
+ mvme5100_request_region(0x20,0x20,"pic1");
+ mvme5100_request_region(0x40,0x20,"timer");
+ mvme5100_request_region(0x80,0x10,"dma page reg");
+ mvme5100_request_region(0xa0,0x20,"pic2");
+ mvme5100_request_region(0xc0,0x20,"dma2");
#endif
return;
}
diff -uprN linux-2.6.16-rc5-orig/arch/ppc/platforms/pplus.c linux-2.6.16-rc5/arch/ppc/platforms/pplus.c
--- linux-2.6.16-rc5-orig/arch/ppc/platforms/pplus.c 2006-02-27
20:36:36.000000000 +0100
+++ linux-2.6.16-rc5/arch/ppc/platforms/pplus.c 2006-02-27 21:59:07.000000000
+0100
@@ -814,17 +814,24 @@ static void __init pplus_map_io(void)
io_block_mapping(0xfef80000, 0xfef80000, 0x00080000, _PAGE_IO);
}

+void __init
+pplus_request_region(unsigned long start, unsigned long n, char *desc) +{
+ if (!request_region(start, n, desc))
+ printk(KERN_WARNING "pplus: unable to allocate %s region.\n", desc);
+}
+
static void __init pplus_init2(void)
{
#ifdef CONFIG_NVRAM
request_region(PREP_NVRAM_AS0, 0x8, "nvram");
#endif
- request_region(0x20, 0x20, "pic1");
- request_region(0xa0, 0x20, "pic2");
- request_region(0x00, 0x20, "dma1");
- request_region(0x40, 0x20, "timer");
- request_region(0x80, 0x10, "dma page reg");
- request_region(0xc0, 0x20, "dma2");
+ pplus_request_region(0x20, 0x20, "pic1");
+ pplus_request_region(0xa0, 0x20, "pic2");
+ pplus_request_region(0x00, 0x20, "dma1");
+ pplus_request_region(0x40, 0x20, "timer");
+ pplus_request_region(0x80, 0x10, "dma page reg");
+ pplus_request_region(0xc0, 0x20, "dma2");
}

/*
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#3 lore raw

PATCH 1/1 check request_region() return value in arch/ppc and

In reply to Tim Cooijmans

From: Tim Cooijmans <[email protected]>

Check request_region() return value and warn on failure. Note that releasing succeeded requests doesn't make much sense, as the calling code doesn't know if anything failed. This is not the case in lopec.c, prep_setup.c and sandpoint.c.

The powerpc code has been compile tested. The ppc code has not been tested, but the changes are very similar.

---
Note: this patch is quite different from the previous one, it also handles request_region() in lopec.c, prep_setup.c and sandpoint.c. There's only one unchecked request_region() left in arch/p{ower,}pc now, in arch/ppc/platforms/hdpu.c:hdpu_ide_request_region(). That function is not used, so I don't know if the return type can be changed.

diff -uprN linux-2.6.16-rc5-orig/arch/powerpc/platforms/chrp/setup.c linux-2.6.16-rc5/arch/powerpc/platforms/chrp/setup.c
--- linux-2.6.16-rc5-orig/arch/powerpc/platforms/chrp/setup.c 2006-02-27
20:36:36.000000000 +0100
+++ linux-2.6.16-rc5/arch/powerpc/platforms/chrp/setup.c 2006-02-28
07:03:41.000000000 +0100
@@ -465,18 +465,25 @@ void __init chrp_init_IRQ(void)
}

void __init
+chrp_request_region(unsigned long start, unsigned long n, char *desc) +{
+ if (!request_region(start, n, desc))
+ printk(KERN_WARNING "chrp: unable to allocate %s region.\n", desc);
+}
+
+void __init
chrp_init2(void)
{
#ifdef CONFIG_NVRAM
chrp_nvram_init();
#endif

- request_region(0x20,0x20,"pic1");
- request_region(0xa0,0x20,"pic2");
- request_region(0x00,0x20,"dma1");
- request_region(0x40,0x20,"timer");
- request_region(0x80,0x10,"dma page reg");
- request_region(0xc0,0x20,"dma2");
+ chrp_request_region(0x20,0x20,"pic1");
+ chrp_request_region(0xa0,0x20,"pic2");
+ chrp_request_region(0x00,0x20,"dma1");
+ chrp_request_region(0x40,0x20,"timer");
+ chrp_request_region(0x80,0x10,"dma page reg");
+ chrp_request_region(0xc0,0x20,"dma2");

if (ppc_md.progress)
ppc_md.progress(" Have fun! ", 0x7777);
diff -uprN linux-2.6.16-rc5-orig/arch/powerpc/platforms/pseries/pci.c linux-2.6.16-rc5/arch/powerpc/platforms/pseries/pci.c
--- linux-2.6.16-rc5-orig/arch/powerpc/platforms/pseries/pci.c 2006-02-27
20:36:36.000000000 +0100
+++ linux-2.6.16-rc5/arch/powerpc/platforms/pseries/pci.c 2006-02-28
06:29:47.000000000 +0100
@@ -92,17 +92,24 @@ void __devinit pSeries_irq_bus_setup(str
}
}

+void __init
+pSeries_request_region(unsigned long start, unsigned long n, char *desc) +{
+ if (!request_region(start, n, desc))
+ printk(KERN_WARNING "pSeries: unable to allocate %s region.\n", desc);
+}
+
static void __init pSeries_request_regions(void)
{
if (!isa_io_base)
return;

- request_region(0x20,0x20,"pic1");
- request_region(0xa0,0x20,"pic2");
- request_region(0x00,0x20,"dma1");
- request_region(0x40,0x20,"timer");
- request_region(0x80,0x10,"dma page reg");
- request_region(0xc0,0x20,"dma2");
+ pSeries_request_region(0x20,0x20,"pic1");
+ pSeries_request_region(0xa0,0x20,"pic2");
+ pSeries_request_region(0x00,0x20,"dma1");
+ pSeries_request_region(0x40,0x20,"timer");
+ pSeries_request_region(0x80,0x10,"dma page reg");
+ pSeries_request_region(0xc0,0x20,"dma2");
}

void __init pSeries_final_fixup(void)
diff -uprN linux-2.6.16-rc5-orig/arch/ppc/platforms/chrp_setup.c linux-2.6.16-rc5/arch/ppc/platforms/chrp_setup.c
--- linux-2.6.16-rc5-orig/arch/ppc/platforms/chrp_setup.c 2006-02-27
20:36:36.000000000 +0100
+++ linux-2.6.16-rc5/arch/ppc/platforms/chrp_setup.c 2006-02-28
07:03:56.000000000 +0100
@@ -450,18 +450,25 @@ void __init chrp_init_IRQ(void)
}

void __init
+chrp_request_region(unsigned long start, unsigned long n, char *desc) +{
+ if (!request_region(start, n, desc))
+ printk(KERN_WARNING "chrp: unable to allocate %s region.\n", desc);
+}
+
+void __init
chrp_init2(void)
{
#ifdef CONFIG_NVRAM
chrp_nvram_init();
#endif

- request_region(0x20,0x20,"pic1");
- request_region(0xa0,0x20,"pic2");
- request_region(0x00,0x20,"dma1");
- request_region(0x40,0x20,"timer");
- request_region(0x80,0x10,"dma page reg");
- request_region(0xc0,0x20,"dma2");
+ chrp_request_region(0x20,0x20,"pic1");
+ chrp_request_region(0xa0,0x20,"pic2");
+ chrp_request_region(0x00,0x20,"dma1");
+ chrp_request_region(0x40,0x20,"timer");
+ chrp_request_region(0x80,0x10,"dma page reg");
+ chrp_request_region(0xc0,0x20,"dma2");

if (ppc_md.progress)
ppc_md.progress(" Have fun! ", 0x7777);
diff -uprN linux-2.6.16-rc5-orig/arch/ppc/platforms/lopec.c linux-2.6.16-rc5/arch/ppc/platforms/lopec.c
--- linux-2.6.16-rc5-orig/arch/ppc/platforms/lopec.c 2006-02-27
20:36:36.000000000 +0100
+++ linux-2.6.16-rc5/arch/ppc/platforms/lopec.c 2006-02-28 06:47:09.000000000
+0100
@@ -274,18 +274,33 @@ lopec_init_IRQ(void)
i8259_init(0xfef00000, 0);
}

+struct resource * __init
+lopec_request_region(unsigned long start, unsigned long n, char *desc) +{
+ struct resource *res = request_region(start, n, desc);
+ if (!res)
+ printk(KERN_WARNING "lopec: unable to allocate %s region.\n", desc);
+ return res;
+}
+
static int __init
lopec_request_io(void)
{
outb(0x00, 0x4d0);
outb(0xc0, 0x4d1);

- request_region(0x00, 0x20, "dma1");
- request_region(0x20, 0x20, "pic1");
- request_region(0x40, 0x20, "timer");
- request_region(0x80, 0x10, "dma page reg");
- request_region(0xa0, 0x20, "pic2");
- request_region(0xc0, 0x20, "dma2");
+ if (!lopec_request_region(0x00, 0x20, "dma1"))
+ return -EBUSY;
+ if (!lopec_request_region(0x20, 0x20, "pic1"))
+ return -EBUSY;
+ if (!lopec_request_region(0x40, 0x20, "timer"))
+ return -EBUSY;
+ if (!lopec_request_region(0x80, 0x10, "dma page reg"))
+ return -EBUSY;
+ if (!lopec_request_region(0xa0, 0x20, "pic2"))
+ return -EBUSY;
+ if (!lopec_request_region(0xc0, 0x20, "dma2"))
+ return -EBUSY;

return 0;
}
diff -uprN linux-2.6.16-rc5-orig/arch/ppc/platforms/mvme5100.c linux-2.6.16-rc5/arch/ppc/platforms/mvme5100.c
--- linux-2.6.16-rc5-orig/arch/ppc/platforms/mvme5100.c 2006-02-27
20:36:36.000000000 +0100
+++ linux-2.6.16-rc5/arch/ppc/platforms/mvme5100.c 2006-02-28
06:29:47.000000000 +0100
@@ -189,16 +189,23 @@ mvme5100_setup_arch(void)
return;
}

+void __init
+mvme5100_request_region(unsigned long start, unsigned long n, char *desc) +{
+ if (!request_region(start, n, desc))
+ printk(KERN_WARNING "mvme5100: unable to allocate %s region.\n", desc);
+}
+
static void __init
mvme5100_init2(void)
{
#ifdef CONFIG_MVME5100_IPMC761_PRESENT
- request_region(0x00,0x20,"dma1");
- request_region(0x20,0x20,"pic1");
- request_region(0x40,0x20,"timer");
- request_region(0x80,0x10,"dma page reg");
- request_region(0xa0,0x20,"pic2");
- request_region(0xc0,0x20,"dma2");
+ mvme5100_request_region(0x00,0x20,"dma1");
+ mvme5100_request_region(0x20,0x20,"pic1");
+ mvme5100_request_region(0x40,0x20,"timer");
+ mvme5100_request_region(0x80,0x10,"dma page reg");
+ mvme5100_request_region(0xa0,0x20,"pic2");
+ mvme5100_request_region(0xc0,0x20,"dma2");
#endif
return;
}
diff -uprN linux-2.6.16-rc5-orig/arch/ppc/platforms/pplus.c linux-2.6.16-rc5/arch/ppc/platforms/pplus.c
--- linux-2.6.16-rc5-orig/arch/ppc/platforms/pplus.c 2006-02-27
20:36:36.000000000 +0100
+++ linux-2.6.16-rc5/arch/ppc/platforms/pplus.c 2006-02-28 06:56:32.000000000
+0100
@@ -814,17 +814,24 @@ static void __init pplus_map_io(void)
io_block_mapping(0xfef80000, 0xfef80000, 0x00080000, _PAGE_IO);
}

+void __init
+pplus_request_region(unsigned long start, unsigned long n, char *desc) +{
+ if (!request_region(start, n, desc))
+ printk(KERN_WARNING "pplus: unable to allocate %s region.\n", desc);
+}
+
static void __init pplus_init2(void)
{
#ifdef CONFIG_NVRAM
- request_region(PREP_NVRAM_AS0, 0x8, "nvram");
+ pplus_request_region(PREP_NVRAM_AS0, 0x8, "nvram");
#endif
- request_region(0x20, 0x20, "pic1");
- request_region(0xa0, 0x20, "pic2");
- request_region(0x00, 0x20, "dma1");
- request_region(0x40, 0x20, "timer");
- request_region(0x80, 0x10, "dma page reg");
- request_region(0xc0, 0x20, "dma2");
+ pplus_request_region(0x20, 0x20, "pic1");
+ pplus_request_region(0xa0, 0x20, "pic2");
+ pplus_request_region(0x00, 0x20, "dma1");
+ pplus_request_region(0x40, 0x20, "timer");
+ pplus_request_region(0x80, 0x10, "dma page reg");
+ pplus_request_region(0xc0, 0x20, "dma2");
}

/*
diff -uprN linux-2.6.16-rc5-orig/arch/ppc/platforms/prep_setup.c linux-2.6.16-rc5/arch/ppc/platforms/prep_setup.c
--- linux-2.6.16-rc5-orig/arch/ppc/platforms/prep_setup.c 2006-02-27
20:36:36.000000000 +0100
+++ linux-2.6.16-rc5/arch/ppc/platforms/prep_setup.c 2006-02-28
06:49:43.000000000 +0100
@@ -1066,17 +1066,31 @@ prep_map_io(void)
io_block_mapping(0xf0000000, PREP_ISA_MEM_BASE, 0x08000000, _PAGE_IO);
}

+struct resource * __init
+prep_request_region(unsigned long start, unsigned long n, char *desc) +{
+ struct resource *res = request_region(start, n, desc);
+ if (!res)
+ printk(KERN_WARNING "prep: unable to allocate %s region.\n", desc);
+ return res;
+}
+
static int __init
prep_request_io(void)
{
if (_machine = _MACH_prep) {
#ifdef CONFIG_NVRAM
- request_region(PREP_NVRAM_AS0, 0x8, "nvram");
+ if (!prep_request_region(PREP_NVRAM_AS0, 0x8, "nvram"))
+ return -EBUSY;
#endif
- request_region(0x00,0x20,"dma1");
- request_region(0x40,0x20,"timer");
- request_region(0x80,0x10,"dma page reg");
- request_region(0xc0,0x20,"dma2");
+ if (!prep_request_region(0x00,0x20,"dma1"))
+ return -EBUSY;
+ if (!prep_request_region(0x40,0x20,"timer"))
+ return -EBUSY;
+ if (!prep_request_region(0x80,0x10,"dma page reg"))
+ return -EBUSY;
+ if (!prep_request_region(0xc0,0x20,"dma2"))
+ return -EBUSY;
}

return 0;
diff -uprN linux-2.6.16-rc5-orig/arch/ppc/platforms/sandpoint.c linux-2.6.16-rc5/arch/ppc/platforms/sandpoint.c
--- linux-2.6.16-rc5-orig/arch/ppc/platforms/sandpoint.c 2006-02-27
20:36:36.000000000 +0100
+++ linux-2.6.16-rc5/arch/ppc/platforms/sandpoint.c 2006-02-28
06:51:53.000000000 +0100
@@ -460,15 +460,30 @@ sandpoint_setup_natl_87308(void)

arch_initcall(sandpoint_setup_natl_87308);

+struct resource * __init
+sandpoint_request_region(unsigned long start, unsigned long n, char *desc) +{
+ struct resource *res = request_region(start, n, desc);
+ if (!res)
+ printk(KERN_WARNING "sandpoint: unable to allocate %s region.\n", desc);
+ return res;
+}
+
static int __init
sandpoint_request_io(void)
{
- request_region(0x00,0x20,"dma1");
- request_region(0x20,0x20,"pic1");
- request_region(0x40,0x20,"timer");
- request_region(0x80,0x10,"dma page reg");
- request_region(0xa0,0x20,"pic2");
- request_region(0xc0,0x20,"dma2");
+ if (!sandpoint_request_region(0x00, 0x20, "dma1"))
+ return -EBUSY;
+ if (!sandpoint_request_region(0x20, 0x20, "pic1"))
+ return -EBUSY;
+ if (!sandpoint_request_region(0x40, 0x20, "timer"))
+ return -EBUSY;
+ if (!sandpoint_request_region(0x80, 0x10, "dma page reg"))
+ return -EBUSY;
+ if (!sandpoint_request_region(0xa0, 0x20, "pic2"))
+ return -EBUSY;
+ if (!sandpoint_request_region(0xc0, 0x20, "dma2"))
+ return -EBUSY;

return 0;
}
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#4 lore raw
In reply to Tim Cooijmans
Quoted 16 lines · Tim Cooijmans

void __init

arch/powerpc/platforms/chrp/setup.c +7 -2
+chrp_request_region(unsigned long start, unsigned long n, char *desc)
+{
+	if (!request_region(start, n, desc))
+		printk(KERN_WARNING "chrp: unable to allocate %s region.\n", desc);
+}
+
+void __init
 chrp_init2(void)
 {
 #ifdef CONFIG_NVRAM
 	chrp_nvram_init();
 #endif
 
-	request_region(0x20,0x20,"pic1");
-	request_region(0xa0,0x20,"pic2");

arch/powerpc/platforms/chrp/setup.c void __init chrp_init_IRQ(void) · line 487

Quoted 10 lines
arch/powerpc/platforms/chrp/setup.c +6 -4
-	request_region(0x00,0x20,"dma1");
-	request_region(0x40,0x20,"timer");
-	request_region(0x80,0x10,"dma page reg");
-	request_region(0xc0,0x20,"dma2");
+	chrp_request_region(0x20,0x20,"pic1");
+	chrp_request_region(0xa0,0x20,"pic2");
+	chrp_request_region(0x00,0x20,"dma1");
+	chrp_request_region(0x40,0x20,"timer");
+	chrp_request_region(0x80,0x10,"dma page reg");
+	chrp_request_region(0xc0,0x20,"dma2");

OK, you've followed this route. One question they'll ask me is

Can those calls ever fail?

I mean, you're deeply in arch-specific setup code. And request_region() parameters are hardwired. And numbers are common among other subarchs in this patch.

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#5 lore raw
In reply to Tim Cooijmans
Quoted 30 lines · Alexey Dobriyan
Quoted 16 lines · Tim Cooijmans

void __init

arch/powerpc/platforms/chrp/setup.c +7 -2
+chrp_request_region(unsigned long start, unsigned long n, char *desc)
+{
+     if (!request_region(start, n, desc))
+             printk(KERN_WARNING "chrp: unable to allocate %s region.\n", desc);
+}
+
+void __init
 chrp_init2(void)
 {
 #ifdef CONFIG_NVRAM
      chrp_nvram_init();
 #endif
 
-     request_region(0x20,0x20,"pic1");
-     request_region(0xa0,0x20,"pic2");

arch/powerpc/platforms/chrp/setup.c void __init chrp_init_IRQ(void) · line 487

Quoted 10 lines
arch/powerpc/platforms/chrp/setup.c +6 -4
-     request_region(0x00,0x20,"dma1");
-     request_region(0x40,0x20,"timer");
-     request_region(0x80,0x10,"dma page reg");
-     request_region(0xc0,0x20,"dma2");
+     chrp_request_region(0x20,0x20,"pic1");
+     chrp_request_region(0xa0,0x20,"pic2");
+     chrp_request_region(0x00,0x20,"dma1");
+     chrp_request_region(0x40,0x20,"timer");
+     chrp_request_region(0x80,0x10,"dma page reg");
+     chrp_request_region(0xc0,0x20,"dma2");

OK, you've followed this route. One question they'll ask me is

Quoted 7 lines

Can those calls ever fail?

I mean, you're deeply in arch-specific setup code. And request_region() parameters are hardwired. And numbers are common among other subarchs in this patch.

I believe the comments Paul Mackerras gave after he NACK'ed a similar patch of mine in -mm explains this very well. The thread is on the mm-commits list, but I've quoted all of it below for your convenience (the interresting bits are at the end) :

---< cut here >---

The patch titled

request_region fixes for ppc prep_request_io

has been added to the -mm tree. Its filename is

request_region-fixes-for-ppc-prep_request_io.patch

See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find out what to do about this

From: Jesper Juhl <[email protected]>

request_region fixes for ppc prep_request_io.

Cc Benjamin Herrenschmidt <[email protected]>
Cc Paul Mackerras <[email protected]>

---

 arch/ppc/platforms/prep_setup.c |   51 +++++++++++++++++++++++++-----
 1 files changed, 44 insertions(+), 7 deletions(-)

diff -puN arch/ppc/platforms/prep_setup.c~request_region-fixes-for-ppc-prep_request_io arch/ppc/platforms/prep_setup.c
--- 25/arch/ppc/platforms/prep_setup.c~request_region-fixes-for-ppc-prep_request_io

    Fri Feb 24 15:10:27 2006
+++ 25-akpm/arch/ppc/platforms/prep_setup.c     Fri Feb 24 15:10:27 2006
@@ -1064,20 +1064,57 @@ prep_map_io(void)
       io_block_mapping(0xf0000000, PREP_ISA_MEM_BASE, 0x08000000, _PAGE_IO);

}

+static inline int
+prep_request_region(unsigned long start, unsigned long n, const char *name) +{
+ struct resource *region;
+
+ region = request_region(start, n, name);
+ if (!region) {
+ printk(KERN_WARNING "prep_request_io: could not allocate "
+ "%s region\n", name);
+ return 0;
+ }
+ return 1;
+}
+
static int __init
prep_request_io(void)
{
- if (_machine = _MACH_prep) {
+ int tmp;
+
+ if (_machine != _MACH_prep)
+ return 0;
+
#ifdef CONFIG_NVRAM
- request_region(PREP_NVRAM_AS0, 0x8, "nvram");
+ tmp = prep_request_region(PREP_NVRAM_AS0, 0x8, "nvram");
+ if (!tmp)
+ goto out_nvram;
#endif

-               request_region(0x00,0x20,"dma1");
-               request_region(0x40,0x20,"timer");
-               request_region(0x80,0x10,"dma page reg");
-               request_region(0xc0,0x20,"dma2");
-       }
+       tmp = prep_request_region(0x00,0x20,"dma1");
+       if (!tmp)

+ goto out_dma1;
+ tmp = prep_request_region(0x40,0x20,"timer");
+ if (!tmp)
+ goto out_timer;
+ tmp = prep_request_region(0x80,0x10,"dma page reg");
+ if (!tmp)
+ goto out_dma_page;
+ tmp = prep_request_region(0xc0,0x20,"dma2");
+ if (!tmp)
+ goto out_dma2;

       return 0;

+ out_dma2:
+ release_region(0x80,0x10);
+ out_dma_page:
+ release_region(0x40,0x20);
+ out_timer:
+ release_region(0x00,0x20);
+ out_dma1:
+ release_region(PREP_NVRAM_AS0, 0x8);
+ out_nvram:
+ return -EBUSY;
}

device_initcall(prep_request_io);
_

Patches currently in -mm which might be from [email protected] are

git-net.patch reduce-nr-of-ptr-derefs-in-fs-jffs2-summaryc.patch isdn-fix-copy_to_user-unused-result-warning-in-isdn_ppp.patch docs-update-missing-files-and-descriptions-for-filesystems-00-index.patch debug-shared-irqs.patch decrease-number-of-pointer-derefs-in-jsm_ttyc.patch sound-remove-unneeded-kmalloc-return-value-casts.patch dont-check-vfree-argument-for-null-in-vx_pcm.patch dont-check-vfree-arg-for-null-in-usbaudio.patch small-whitespace-cleanup-for-qlogic-driver.patch dont-null-check-vfree-argument-in-pdaudiocf_pcm.patch request_region-fixes-for-ppc-prep_request_io.patch vfree-null-check-fixup-for-sb_card.patch maestro3-vfree-null-check-fixup.patch ne2000-kconfig-help-entry-improvement.patch no-need-to-check-vfree-arg-for-null-in-oss-sequencer.patch vfree-does-its-own-null-check-no-need-to-be-explicit-in-oss-msndc.patch fix-signed-vs-unsigned-in-nmi-watchdog.patch const-static-vs-static-const-in-nfs4.patch trivial-typos-in-documentation-cputopologytxt.patch

Reply Reply to all Forward Invite [email protected] to Gmail

Paul Mackerras
to akpm, me, benh, mm-commits
More options Feb 26 (2 days ago)

Quoted 10 lines · [email protected] writes:

The patch titled

request_region fixes for ppc prep_request_io

has been added to the -mm tree. Its filename is

request_region-fixes-for-ppc-prep_request_io.patch

See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find out what to do about this

Nack. Printing a message if one of the request_regions fails is OK, but why release all the previous regions, and why not go on to request the rest?

Review

Quoted 3 lines
(quoted diff) +3 -0
+     tmp = prep_request_region(0x00,0x20,"dma1");
+     if (!tmp)
+             goto out_dma1;

Use of the tmp variable is unnecessary and verbose.

Paul.

Reply Reply to all Forward Invite Paul to Gmail

Jesper Juhl
to Paul, akpm, benh, mm-commits
More options Feb 26 (2 days ago)

Quoted 6 lines · Paul Mackerras
Quoted 4 lines · [email protected] writes:

The patch titled

request_region fixes for ppc prep_request_io

Quoted 8 lines
Quoted 6 lines

has been added to the -mm tree. Its filename is

request_region-fixes-for-ppc-prep_request_io.patch

See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find out what to do about this

Nack.

Thanks. I appreciate you taking the time to review the patch.

Quoted 1 line

Printing a message if one of the request_regions fails is OK,

OK.

Quoted 2 lines

but why release all the previous regions, and why not go on to request the rest?

Probably because I misunderstood the code.

As I read it, all regions must be requested successfully or none at all, and in that case (one or more failing) we should report an error back.

But from your comment it is clear that that is a misunderstanding. Would you mind enlightening me on the purpose of the code and what will happen if one or more of the request_regions fail but we just continue regardless ?

Quoted 7 lines

Review

Quoted 4 lines
(quoted diff) +3 -0
+     tmp = prep_request_region(0x00,0x20,"dma1");
+     if (!tmp)
+             goto out_dma1;

Use of the tmp variable is unnecessary and verbose.

True.

--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

Reply Reply to all Forward

Paul Mackerras
to me, akpm, benh, mm-commits
More options 7:10 am (15 hours ago)

Quoted 8 lines · Jesper Juhl
Quoted 2 lines

but why release all the previous regions, and why not go on to request the rest?

Probably because I misunderstood the code.

As I read it, all regions must be requested successfully or none at all, and in that case (one or more failing) we should report an error back.

Well, returning an error code from an initcall function doesn't actually do anything.

In the case of this code, it isn't that there is some driver which can refuse to load if an error is reported, since these are essential platform functions that the I/O port ranges are being reserved for (DMA controller, timer, nvram). The code that uses these devices is elsewhere and doesn't change its behaviour if the request_region calls fail.

The main reasons for doing the request_region calls is so that the user sees the ranges marked as reserved in /proc/ioports, and to stop any other driver from requesting those regions.

Quoted 4 lines

But from your comment it is clear that that is a misunderstanding. Would you mind enlightening me on the purpose of the code and what will happen if one or more of the request_regions fail but we just continue regardless ?

That's a good question. The only reasons I can think of for the request_region calls failing are that there is some kernel bug causing the request to fail even though nothing else has done an overlapping request, or that some buggy driver has requested an overlapping region. In the first case, the code that uses the DMA controller, timer or nvram should still work fine.

In the second case, it depends on whether the buggy driver is also poking I/O ports in the region(s) it has requested. (The driver must be buggy, because a PReP platform would never have an actual device with I/O port regions that overlapped these system devices.) If the driver has just requested the wrong ports but is doing I/O to the correct ports, then things will still work. If the driver is doing I/O to the wrong ports then anything can happen.

There is an argument that if there is the possibility that some driver is doing I/O to the wrong ports, we should just panic to avoid the possibility of data corruption. On the other hand there is the argument that we should try to continue running if possible.

Actual PReP machines are pretty old by now, since it is an old specification that has been superseded. There are possibly embedded PPC boards designed to the PReP spec still being made, though. Given that, I tend to lean towards the "continue running" side of the argument. (Also, continuing to run will mean that the user can look in /proc/ioports and see what has done the interfering request.)

Paul.

--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#6 lore raw
In reply to Tim Cooijmans
Quoted 7 lines · Alexey Dobriyan

OK, you've followed this route. One question they'll ask me is

Can those calls ever fail?

I mean, you're deeply in arch-specific setup code. And request_region() parameters are hardwired. And numbers are common among other subarchs in this patch.

That's a good question. Jesper's e-mail explains it well, too. So, no request_region checking in /arch? Should it only be done in drivers?

Tim
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors