[KJ] pci-sh7751.c request_region return value check

[KJ] pci-sh7751.c request_region return value check

[Carlos Manuel Duclos Vergara]

[-- Attachment #1: Type: text/plain, Size: 485 bytes --]

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

-- 
Carlos Manuel Duclos Vergara
http://www.toolchains.com/personal/blog

[-- Attachment #2: pci-sh7751.c.patch --]
[-- Type: text/x-diff, Size: 1012 bytes --]

--- Builds/linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c	2005-11-24 19:10:21.000000000 -0300
+++ Devel/linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c	2005-11-26 19:51:29.000000000 -0300
@@ -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));

[-- Attachment #3: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] pci-sh7751.c request_region return value check

[Matthew Wilcox]

[-- Attachment #1: Type: text/plain, Size: 387 bytes --]

On Sat, Nov 26, 2005 at 08:00:11PM -0300, Carlos Manuel Duclos Vergara wrote:
>  	unsigned int tmp, id;
> +	struct resource * r = NULL;

CodingStyle says no space between the * and r.

> +			if( r == NULL )
> +			{

"if (!r) {

> +				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_


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] pci-sh7751.c request_region return value check

[Carlos Manuel Duclos Vergara]

[-- Attachment #1: Type: text/plain, Size: 134 bytes --]

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

-- 
Carlos Manuel Duclos Vergara
http://www.toolchains.com/personal/blog

[-- Attachment #2: pci-sh7751.c.patch --]
[-- Type: text/x-diff, Size: 1003 bytes --]

--- Builds/linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c	2005-11-24 19:10:21.000000000 -0300
+++ Devel/linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c	2005-11-27 14:07:39.000000000 -0300
@@ -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));

[-- Attachment #3: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] pci-sh7751.c request_region return value check

[Nishanth Aravamudan]

[-- Attachment #1: Type: text/plain, Size: 1191 bytes --]

On 27.11.2005 [14:09:49 -0300], Carlos Manuel Duclos Vergara wrote:
> Ok, thanks for your corrections. Attached is the new patch.
> 
> -- 
> Carlos Manuel Duclos Vergara
> http://www.toolchains.com/personal/blog

> --- Builds/linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c	2005-11-24 19:10:21.000000000 -0300
> +++ Devel/linux-2.6.14.3/arch/sh/drivers/pci/pci-sh7751.c	2005-11-27 14:07:39.000000000 -0300

-p1 applicable patches, please.

> @@ -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 )
> +			{

	if (!r) {

as Matthew already pointed out.

Please read Documentation/CodingStyle.

Thanks,
Nish

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors