Skip to contentSkip to conversation

pci-sh7751.c request_region return value check

KJ Carlos Manuel Duclos Vergara 3 participants 4 messages Nov 26, 2005 – Nov 27, 2005 Kernel janitors

lore
Share & tools
lore

Carlos Manuel Duclos Vergara

author
#0 lore raw

Hi,

following your advises in your web site this is a small patch because is the first patch I submit to this project.
The request_region value was not checked in the function pci_check_direct(...), so I took care of it and in case of problems I return ENODEV instead of always returning 0 (as it does now).
Any comments, suggestions, etc... are welcomed.

Cheers!

PS This patch is against kernel 2.6.14.3

Signature · 2 lines
Carlos Manuel Duclos Vergara
http://www.toolchains.com/personal/blog
linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c +11 -1
@@ -126,6 +126,7 @@
 static int __init pci_check_direct(void)
 {
 	unsigned int tmp, id;
+	struct resource * r = NULL;
 
 	/* check for SH7751/SH7751R hardware */
 	id = inl(SH7751_PCIREG_BASE+SH7751_PCICONF0);

@@ -144,7 +145,16 @@
 		if (inl (PCI_REG(SH7751_PCIPAR)) == 0x80000000) {
 			outl (tmp, PCI_REG(SH7751_PCIPAR));
 			printk(KERN_INFO "PCI: Using configuration type 1\n");
-			request_region(PCI_REG(SH7751_PCIPAR), 8, "PCI conf1");
+			r = request_region(PCI_REG(SH7751_PCIPAR), 8, "PCI conf1");
+			/*
+			 * Murphy's law....
+			 * (ENODEV or EINVAL in case of problems?)
+			 */
+			if( r == NULL )
+			{
+				printk( KERN_CRIT "PCI: Could not allocate memory for our controller (Address: %lu)\n", PCI_REG(SH7751_PCIPAR));
+				return -ENODEV;
+			}
 			return 0;
 		}
 		outl (tmp, PCI_REG(SH7751_PCIPAR));

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

#1 lore raw

linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c line 129

Quoted 2 lines · Carlos Manuel Duclos Vergara
linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c +1 -0
 	unsigned int tmp, id;
+	struct resource * r = NULL;

CodingStyle says no space between the * and r.

linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c line 154

Quoted 2 lines
linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c +2 -0
+			if( r == NULL )
+			{

"if (!r) {

linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c line 155

Quoted 1 line
linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c +1 -0
+				printk( KERN_CRIT "PCI: Could not allocate memory for our controller (Address: %lu)\n", PCI_REG(SH7751_PCIPAR));

No space between the ( and the KERN_

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

author
#2 lore raw
In reply to Carlos Manuel Duclos Vergara

Ok, thanks for your corrections. Attached is the new patch.

Signature · 2 lines
Carlos Manuel Duclos Vergara
http://www.toolchains.com/personal/blog
linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c +11 -1
@@ -126,6 +126,7 @@
 static int __init pci_check_direct(void)
 {
 	unsigned int tmp, id;
+	struct resource *r = NULL;
 
 	/* check for SH7751/SH7751R hardware */
 	id = inl(SH7751_PCIREG_BASE+SH7751_PCICONF0);

@@ -144,7 +145,16 @@
 		if (inl (PCI_REG(SH7751_PCIPAR)) == 0x80000000) {
 			outl (tmp, PCI_REG(SH7751_PCIPAR));
 			printk(KERN_INFO "PCI: Using configuration type 1\n");
-			request_region(PCI_REG(SH7751_PCIPAR), 8, "PCI conf1");
+			r = request_region(PCI_REG(SH7751_PCIPAR), 8, "PCI conf1");
+			/*
+			 * Murphy's law....
+			 * (ENODEV or EINVAL in case of problems?)
+			 */
+			if( !r )
+			{
+				printk(KERN_CRIT "PCI: Could not allocate memory for our controller (Address: %lu)\n", PCI_REG(SH7751_PCIPAR));
+				return -ENODEV;
+			}
 			return 0;
 		}
 		outl (tmp, PCI_REG(SH7751_PCIPAR));

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

#3 lore raw
In reply to Carlos Manuel Duclos Vergara

linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c

Quoted 8 lines · Carlos Manuel Duclos Vergara

Ok, thanks for your corrections. Attached is the new patch.

Signature · 2 lines
Carlos Manuel Duclos Vergara
http://www.toolchains.com/personal/blog
linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c +0 -0

-p1 applicable patches, please.

Quoted 10 lines
linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c +1 -0
@@ -126,6 +126,7 @@
 static int __init pci_check_direct(void)
 {
 	unsigned int tmp, id;
+	struct resource *r = NULL;
 
 	/* check for SH7751/SH7751R hardware */
 	id = inl(SH7751_PCIREG_BASE+SH7751_PCICONF0);

@@ -144,7 +145,16 @@
 		if (inl (PCI_REG(SH7751_PCIPAR)) == 0x80000000) {

linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c line 154

Quoted 10 lines
linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c +7 -1
 			outl (tmp, PCI_REG(SH7751_PCIPAR));
 			printk(KERN_INFO "PCI: Using configuration type 1\n");
-			request_region(PCI_REG(SH7751_PCIPAR), 8, "PCI conf1");
+			r = request_region(PCI_REG(SH7751_PCIPAR), 8, "PCI conf1");
+			/*
+			 * Murphy's law....
+			 * (ENODEV or EINVAL in case of problems?)
+			 */
+			if( !r )
+			{

	if (!r) {

as Matthew already pointed out.

Please read Documentation/CodingStyle.

Thanks,
Nish

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