dgrs - Fixes Warnings when CONFIG_ISA and CONFIG_PCI are not enabled

PATCH In review Ashutosh Naik 4 participants 15 messages Nov 2, 2005 – Nov 5, 2005

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Ashutosh Naik

author

2005-11-02 07:14 UTC #0 lore raw

PATCH 1/1 dgrs - Fixes Warnings when CONFIG_ISA and CONFIG_PCI are not enabled

This patch fixes compiler warnings when CONFIG_ISA and CONFIG_PCI are not enabled in the dgrc network driver.

Signed-off-by Ashutosh Naik <[email protected]>

--
diff -Naurp linux-2.6.14/drivers/net/dgrs.c linux-2.6.14-git1/drivers/net/dgrs.c---
linux-2.6.14/drivers/net/dgrs.c 2005-10-28 05:32:08.000000000
+0530
+++ linux-2.6.14-git1/drivers/net/dgrs.c 2005-11-01
10:30:03.000000000 +0530
@@ -1549,8 +1549,12 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi

 static int __init dgrs_init_module (void)  {
        int     i;

- int eisacount = 0, pcicount = 0;
-
+#ifdef CONFIG_EISA
+ int eisacount = 0;
+#endif
+#ifdef CONFIG_PCI
+ int pcicount = 0;
+#endif
/*
* Command line variable overrides
* debug=NNN

Richard Knutsson

2005-11-02 09:28 UTC #1 lore raw

Quoted 19 lines · Ashutosh Naik

This patch fixes compiler warnings when CONFIG_ISA and CONFIG_PCI are not enabled in the dgrc network driver.
Signed-off-by Ashutosh Naik <[email protected]>
--
diff -Naurp linux-2.6.14/drivers/net/dgrs.c
linux-2.6.14-git1/drivers/net/dgrs.c---
linux-2.6.14/drivers/net/dgrs.c 2005-10-28 05:32:08.000000000
+0530
+++ linux-2.6.14-git1/drivers/net/dgrs.c 2005-11-01
10:30:03.000000000 +0530
(quoted diff) +0 -2
@@ -1549,8 +1549,12 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
 static int __init dgrs_init_module (void)  {
       int     i;
-       int eisacount = 0, pcicount = 0;
-

Review MODULE_PARM_DESC(nicmode, "Digi RightSwi · line 1556

Quoted 10 lines

(quoted diff) +6 -1

+#ifdef CONFIG_EISA
+       int eisacount = 0;
+#endif
+#ifdef CONFIG_PCI
+       int pcicount = 0;
+#endif
       /*
        *      Command line variable overrides
        *              debug=NNN
-

Since eisacount and pcicount is doing the same task (and they are only used in sequence) and to preventing more #ifdef in the source-code, why not use the same variable? It will give an warning if both of them is not defined, but is that an issue? If so, #if !defined CONFIG_EISA && !defined CONFIG_PCI
could encapsulate the variable to prevent that.

Posted 26'th of October and now also checked against 2.6.14-git1.

Signed-off-by Richard Knutsson <[email protected]>

---

drivers/net/dgrs.c +0 -0

diff -uNr a/drivers/net/dgrs.c b/drivers/net/dgrs.c
@@ -1549,7 +1549,7 @@

static int __init dgrs_init_module (void)

{
	int	i;

- int eisacount = 0, pcicount = 0;
+ int count;

	/*
	 *	Command line variable overrides
@@ -1591,14 +1591,14 @@
	 *	Find and configure all the cards
	 */

#ifdef CONFIG_EISA
- eisacount = eisa_driver_register(&dgrs_eisa_driver);
- if (eisacount < 0)
- return eisacount;
+ count = eisa_driver_register(&dgrs_eisa_driver);
+ if (count < 0)
+ return count;
#endif
#ifdef CONFIG_PCI
- pcicount = pci_register_driver(&dgrs_pci_driver);
- if (pcicount)
- return pcicount;
+ count = pci_register_driver(&dgrs_pci_driver);
+ if (count)
+ return count;

#endif
	return 0;
}

Ashutosh Naik

2005-11-02 13:16 UTC #2 lore raw

Quoted 67 lines · Richard Knutsson

Quoted 19 lines · Ashutosh Naik
This patch fixes compiler warnings when CONFIG_ISA and CONFIG_PCI are not enabled in the dgrc network driver.
Signed-off-by Ashutosh Naik <[email protected]>
--
diff -Naurp linux-2.6.14/drivers/net/dgrs.c
linux-2.6.14-git1/drivers/net/dgrs.c---
linux-2.6.14/drivers/net/dgrs.c 2005-10-28 05:32:08.000000000
+0530
+++ linux-2.6.14-git1/drivers/net/dgrs.c 2005-11-01
10:30:03.000000000 +0530
(quoted diff) +0 -2
@@ -1549,8 +1549,12 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
 static int __init dgrs_init_module (void)  {
       int     i;
-       int eisacount = 0, pcicount = 0;
-
Review MODULE_PARM_DESC(nicmode, "Digi RightSwi · line 1556
Quoted 10 lines
(quoted diff) +6 -1
+#ifdef CONFIG_EISA
+       int eisacount = 0;
+#endif
+#ifdef CONFIG_PCI
+       int pcicount = 0;
+#endif
       /*
        *      Command line variable overrides
        *              debug=NNN
-
Since eisacount and pcicount is doing the same task (and they are only used in sequence) and to preventing more #ifdef in the source-code, why not use the same variable? It will give an warning if both of them is not defined, but is that an issue? If so,
#if !defined CONFIG_EISA && !defined CONFIG_PCI
could encapsulate the variable to prevent that.
Posted 26'th of October and now also checked against 2.6.14-git1.
Signed-off-by Richard Knutsson <[email protected]>
---
diff -uNr a/drivers/net/dgrs.c b/drivers/net/dgrs.c
drivers/net/dgrs.c 2005-10-26 15:53:43.000000000 +0200 renamed from drivers/net/dgrs.c 2005-08-29 01:41:01.000000000 +0200 +4 -4
@@ -1549,7 +1549,7 @@
 static int __init dgrs_init_module (void)
 {
        int     i;
-       int eisacount = 0, pcicount = 0;
+       int     count;
 
        /*
         *      Command line variable overrides
@@ -1591,14 +1591,14 @@
         *      Find and configure all the cards
         */
 #ifdef CONFIG_EISA
-       eisacount = eisa_driver_register(&dgrs_eisa_driver);
-       if (eisacount < 0)
-               return eisacount;
+       count = eisa_driver_register(&dgrs_eisa_driver);
+       if (count < 0)
+               return count;
 #endif

drivers/net/dgrs.c 2005-10-26 15:53:43.000000000 +0200 line 1604

Quoted 10 lines

drivers/net/dgrs.c 2005-10-26 15:53:43.000000000 +0200 renamed from drivers/net/dgrs.c 2005-08-29 01:41:01.000000000 +0200 +3 -3
 #ifdef CONFIG_PCI
-       pcicount = pci_register_driver(&dgrs_pci_driver);
-       if (pcicount)
-               return pcicount;
+       count = pci_register_driver(&dgrs_pci_driver);
+       if (count)
+               return count;
 #endif
        return 0;
 }

Well, both of them do the same stuff, but one of these patches needs to be committed.

Cheers
Ashutosh

Richard Knutsson

2005-11-02 20:51 UTC #3 lore raw

Quoted 75 lines

Quoted 61 lines
Quoted 21 lines
This patch fixes compiler warnings when CONFIG_ISA and CONFIG_PCI are not enabled in the dgrc network driver.
Signed-off-by Ashutosh Naik <[email protected]>
--
diff -Naurp linux-2.6.14/drivers/net/dgrs.c
linux-2.6.14-git1/drivers/net/dgrs.c---
linux-2.6.14/drivers/net/dgrs.c 2005-10-28 05:32:08.000000000
+0530
+++ linux-2.6.14-git1/drivers/net/dgrs.c 2005-11-01
10:30:03.000000000 +0530
(quoted diff) +0 -2
@@ -1549,8 +1549,12 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
 static int __init dgrs_init_module (void)  {
      int     i;
-       int eisacount = 0, pcicount = 0;
-
Review MODULE_PARM_DESC(nicmode, "Digi RightSwi · line 1556
Quoted 10 lines
(quoted diff) +6 -1
+#ifdef CONFIG_EISA
+       int eisacount = 0;
+#endif
+#ifdef CONFIG_PCI
+       int pcicount = 0;
+#endif
      /*
       *      Command line variable overrides
       *              debug=NNN
-
Signed-off-by Richard Knutsson <[email protected]>
---
diff -uNr a/drivers/net/dgrs.c b/drivers/net/dgrs.c
drivers/net/dgrs.c 2005-10-26 15:53:43.000000000 +0200 renamed from drivers/net/dgrs.c 2005-08-29 01:41:01.000000000 +0200 +4 -4
@@ -1549,7 +1549,7 @@
 static int __init dgrs_init_module (void)
 {
       int     i;
-       int eisacount = 0, pcicount = 0;
+       int     count;
 
       /*
        *      Command line variable overrides
@@ -1591,14 +1591,14 @@
        *      Find and configure all the cards
        */
 #ifdef CONFIG_EISA
-       eisacount = eisa_driver_register(&dgrs_eisa_driver);
-       if (eisacount < 0)
-               return eisacount;
+       count = eisa_driver_register(&dgrs_eisa_driver);
+       if (count < 0)
+               return count;
 #endif
drivers/net/dgrs.c 2005-10-26 15:53:43.000000000 +0200 line 1604
Quoted 10 lines
drivers/net/dgrs.c 2005-10-26 15:53:43.000000000 +0200 renamed from drivers/net/dgrs.c 2005-08-29 01:41:01.000000000 +0200 +3 -3
 #ifdef CONFIG_PCI
-       pcicount = pci_register_driver(&dgrs_pci_driver);
-       if (pcicount)
-               return pcicount;
+       count = pci_register_driver(&dgrs_pci_driver);
+       if (count)
+               return count;
 #endif
       return 0;
 }
Well, both of them do the same stuff, but one of these patches needs to be committed.

Quoted 4 lines

Cheers
Ashutosh

Can both CONFIG_PCI and CONFIG_EISA be undefined at the same time? If so, I think you patch is better. But on line 1015 in the dgrs.c-file (function dgrs_download()) there is an if-statement:
if (priv0->plxreg)
{ /* PCI bus */

       ...
   }
   else
   {    /* EISA bus */
       ...

from where I got the idea it needs either pci or eisa (or both). If this is true, I vote for my patch.

Live long and prosper
/Richard

PS
Rick's mail-address in the file seems invalid. Changed it to [email protected], since that is the address in the MAINTAINERS-file. DS

Ashutosh Naik

2005-11-03 05:39 UTC #4 lore raw

Quoted 90 lines · Richard Knutsson

Quoted 75 lines
Quoted 61 lines
Quoted 21 lines
This patch fixes compiler warnings when CONFIG_ISA and CONFIG_PCI are not enabled in the dgrc network driver.
Signed-off-by Ashutosh Naik <[email protected]>
--
diff -Naurp linux-2.6.14/drivers/net/dgrs.c
linux-2.6.14-git1/drivers/net/dgrs.c---
linux-2.6.14/drivers/net/dgrs.c 2005-10-28 05:32:08.000000000
+0530
+++ linux-2.6.14-git1/drivers/net/dgrs.c 2005-11-01
10:30:03.000000000 +0530
(quoted diff) +0 -2
@@ -1549,8 +1549,12 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
 static int __init dgrs_init_module (void)  {
      int     i;
-       int eisacount = 0, pcicount = 0;
-
Review MODULE_PARM_DESC(nicmode, "Digi RightSwi · line 1556
Quoted 10 lines
(quoted diff) +6 -1
+#ifdef CONFIG_EISA
+       int eisacount = 0;
+#endif
+#ifdef CONFIG_PCI
+       int pcicount = 0;
+#endif
      /*
       *      Command line variable overrides
       *              debug=NNN
-
Signed-off-by Richard Knutsson <[email protected]>
---
diff -uNr a/drivers/net/dgrs.c b/drivers/net/dgrs.c
drivers/net/dgrs.c 2005-10-26 15:53:43.000000000 +0200 renamed from drivers/net/dgrs.c 2005-08-29 01:41:01.000000000 +0200 +4 -4
@@ -1549,7 +1549,7 @@
 static int __init dgrs_init_module (void)
 {
       int     i;
-       int eisacount = 0, pcicount = 0;
+       int     count;
 
       /*
        *      Command line variable overrides
@@ -1591,14 +1591,14 @@
        *      Find and configure all the cards
        */
 #ifdef CONFIG_EISA
-       eisacount = eisa_driver_register(&dgrs_eisa_driver);
-       if (eisacount < 0)
-               return eisacount;
+       count = eisa_driver_register(&dgrs_eisa_driver);
+       if (count < 0)
+               return count;
 #endif
drivers/net/dgrs.c 2005-10-26 15:53:43.000000000 +0200 line 1604
Quoted 10 lines
drivers/net/dgrs.c 2005-10-26 15:53:43.000000000 +0200 renamed from drivers/net/dgrs.c 2005-08-29 01:41:01.000000000 +0200 +3 -3
 #ifdef CONFIG_PCI
-       pcicount = pci_register_driver(&dgrs_pci_driver);
-       if (pcicount)
-               return pcicount;
+       count = pci_register_driver(&dgrs_pci_driver);
+       if (count)
+               return count;
 #endif
       return 0;
 }
Well, both of them do the same stuff, but one of these patches needs to be committed.
Quoted 4 lines
Cheers
Ashutosh
Can both CONFIG_PCI and CONFIG_EISA be undefined at the same time? If so, I think you patch is better.
But on line 1015 in the dgrs.c-file (function dgrs_download()) there is an if-statement:
    if (priv0->plxreg)
    {    /* PCI bus */
        ...
    }
    else
    {    /* EISA bus */
        ...

Quoted 2 lines

from where I got the idea it needs either pci or eisa (or both). If this is true, I vote for my patch.

Both CONFIG_PCI and CONFIG_EISA cant be undefined at the same time, because the device has to be on either of the 2 busses. I think your patch is better in that case.

Cheers
Ashutosh

Richard Knutsson

2005-11-03 23:09 UTC #5 lore raw

PATCH 1/1 /drivers/net/dgrs.c - Fixes Warnings when CONFIG_EISA or CONFIG_PCI are not enabled

Quoted 34 lines

Quoted 21 lines
Quoted 21 lines
Quoted 21 lines
Quoted 21 lines
This patch fixes compiler warnings when CONFIG_ISA and CONFIG_PCI are not enabled in the dgrc network driver.
Signed-off-by Ashutosh Naik <[email protected]>
--
diff -Naurp linux-2.6.14/drivers/net/dgrs.c
linux-2.6.14-git1/drivers/net/dgrs.c---
linux-2.6.14/drivers/net/dgrs.c 2005-10-28 05:32:08.000000000
+0530
+++ linux-2.6.14-git1/drivers/net/dgrs.c 2005-11-01
10:30:03.000000000 +0530
(quoted diff) +3 -2
@@ -1549,8 +1549,12 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
 static int __init dgrs_init_module (void)  {
     int     i;
-       int eisacount = 0, pcicount = 0;
-
+#ifdef CONFIG_EISA
+       int eisacount = 0;
+#endif
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Review MODULE_PARM_DESC(nicmode, "Digi RightSwi · line 1560
Quoted 8 lines
(quoted diff) +3 -0
+#ifdef CONFIG_PCI
+       int pcicount = 0;
+#endif
     /*
      *      Command line variable overrides
      *              debug=NNN
 
Both CONFIG_PCI and CONFIG_EISA cant be undefined at the same time, because the device has to be on either of the 2 busses. I think your patch is better in that case.

Quoted 4 lines

Cheers
Ashutosh

OK, then I send in the patch again. Thanks for your help/opinion.

Till the next time...
/Richard

<-- snip -->

This patch fixes compiler warnings when CONFIG_ISA or CONFIG_PCI are not enabled in the dgrc network driver.

Cleanly patched to 2.6.14-git6.

Signed-off-by Richard Knutsson <[email protected]>

---

drivers/net/dgrs.c +0 -0

diff -Nurp a/drivers/net/dgrs.c b/drivers/net/dgrs.c
@@ -1549,7 +1549,7 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi

static int __init dgrs_init_module (void)

{
	int	i;

- int eisacount = 0, pcicount = 0;
+ int count;

	/*
	 *	Command line variable overrides
@@ -1591,14 +1591,14 @@ static int __init dgrs_init_module (void
	 *	Find and configure all the cards
	 */

#endif
	return 0;
}

Andrew Morton

2005-11-05 02:25 UTC #6 lore raw

In reply to Richard Knutsson

Quoted 76 lines · Richard Knutsson

Quoted 76 lines
Quoted 62 lines
Quoted 22 lines
This patch fixes compiler warnings when CONFIG_ISA and CONFIG_PCI are not enabled in the dgrc network driver.
Signed-off-by Ashutosh Naik <[email protected]>
--
diff -Naurp linux-2.6.14/drivers/net/dgrs.c
linux-2.6.14-git1/drivers/net/dgrs.c---
linux-2.6.14/drivers/net/dgrs.c 2005-10-28 05:32:08.000000000
+0530
+++ linux-2.6.14-git1/drivers/net/dgrs.c 2005-11-01
10:30:03.000000000 +0530
(quoted diff) +0 -2
@@ -1549,8 +1549,12 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
 static int __init dgrs_init_module (void)  {
      int     i;
-       int eisacount = 0, pcicount = 0;
-
Review MODULE_PARM_DESC(nicmode, "Digi RightSwi · line 1556
Quoted 10 lines
(quoted diff) +6 -1
+#ifdef CONFIG_EISA
+       int eisacount = 0;
+#endif
+#ifdef CONFIG_PCI
+       int pcicount = 0;
+#endif
      /*
       *      Command line variable overrides
       *              debug=NNN
-
Signed-off-by Richard Knutsson <[email protected]>
---
diff -uNr a/drivers/net/dgrs.c b/drivers/net/dgrs.c
drivers/net/dgrs.c 2005-10-26 15:53:43.000000000 +0200 renamed from drivers/net/dgrs.c 2005-08-29 01:41:01.000000000 +0200 +4 -4
@@ -1549,7 +1549,7 @@
 static int __init dgrs_init_module (void)
 {
       int     i;
-       int eisacount = 0, pcicount = 0;
+       int     count;
 
       /*
        *      Command line variable overrides
@@ -1591,14 +1591,14 @@
        *      Find and configure all the cards
        */
 #ifdef CONFIG_EISA
-       eisacount = eisa_driver_register(&dgrs_eisa_driver);
-       if (eisacount < 0)
-               return eisacount;
+       count = eisa_driver_register(&dgrs_eisa_driver);
+       if (count < 0)
+               return count;
 #endif
drivers/net/dgrs.c 2005-10-26 15:53:43.000000000 +0200 line 1604
Quoted 10 lines
drivers/net/dgrs.c 2005-10-26 15:53:43.000000000 +0200 renamed from drivers/net/dgrs.c 2005-08-29 01:41:01.000000000 +0200 +3 -3
 #ifdef CONFIG_PCI
-       pcicount = pci_register_driver(&dgrs_pci_driver);
-       if (pcicount)
-               return pcicount;
+       count = pci_register_driver(&dgrs_pci_driver);
+       if (count)
+               return count;
 #endif
       return 0;
 }
Well, both of them do the same stuff, but one of these patches needs to be committed.

Quoted 6 lines

Quoted 4 lines
Cheers
Ashutosh
Can both CONFIG_PCI and CONFIG_EISA be undefined at the same time? If so

Not for this driver. From drivers/net/dgrs.c:

config DGRS

	tristate "Digi Intl. RightSwitch SE-X support"
	depends on NET_PCI && (PCI || EISA)

Quoted 1 line

I think you patch is better.

Let's go with Ashutosh's patch then, thanks.

Andrew Morton

2005-11-05 02:30 UTC #7 lore raw

Quoted 2 lines · Andrew Morton

Let's go with Ashutosh's patch then, thanks.

(It was wordwrapped. Please fix your email client)

In fact we can de-ifdef things a bit.

drivers/net/dgrs.c renamed from drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled +7 -9

diff -puN drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled drivers/net/dgrs.c
@@ -1549,7 +1549,7 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
 static int __init dgrs_init_module (void)
 {
 	int	i;
-	int eisacount = 0, pcicount = 0;
+	int	cardcount = 0;
 
 	/*
 	 *	Command line variable overrides

@@ -1591,15 +1591,13 @@ static int __init dgrs_init_module (void
 	 *	Find and configure all the cards
 	 */
 #ifdef CONFIG_EISA
-	eisacount = eisa_driver_register(&dgrs_eisa_driver);
-	if (eisacount < 0)
-		return eisacount;
-#endif
-#ifdef CONFIG_PCI
-	pcicount = pci_register_driver(&dgrs_pci_driver);
-	if (pcicount)
-		return pcicount;
+	cardcount = eisa_driver_register(&dgrs_eisa_driver);
+	if (cardcount < 0)
+		return cardcount;
 #endif
+	cardcount = pci_register_driver(&dgrs_pci_driver);
+	if (cardcount)
+		return cardcount;
 	return 0;
 }

Ashutosh Naik

2005-11-05 04:51 UTC #8 lore raw

Hi Andrew,

Quoted 5 lines · Andrew Morton

Quoted 2 lines · Andrew Morton
Let's go with Ashutosh's patch then, thanks.
(It was wordwrapped. Please fix your email client)

I am attaching the patch.

Signed-off-by Ashutosh Naik <[email protected]>

Quoted 26 lines

In fact we can de-ifdef things a bit.

diff -puN drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled drivers/net/dgrs.c

drivers/net/dgrs.c 2005-11-04 18:29:24.000000000 -0800 renamed from drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled 2005-11-04 18:26:59.000000000 -0800 +1 -9

@@ -1549,7 +1549,7 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
 static int __init dgrs_init_module (void)
 {
        int     i;
-       int eisacount = 0, pcicount = 0;
+       int     cardcount = 0;
 
        /*
         *      Command line variable overrides

@@ -1591,15 +1591,13 @@ static int __init dgrs_init_module (void
         *      Find and configure all the cards
         */
 #ifdef CONFIG_EISA
-       eisacount = eisa_driver_register(&dgrs_eisa_driver);
-       if (eisacount < 0)
-               return eisacount;
-#endif
-#ifdef CONFIG_PCI
-       pcicount = pci_register_driver(&dgrs_pci_driver);
-       if (pcicount)
-               return pcicount;

drivers/net/dgrs.c 2005-11-04 18:29:24.000000000 -0800 static int __init dgrs_init_module (void · line 1603

Quoted 10 lines

drivers/net/dgrs.c 2005-11-04 18:29:24.000000000 -0800 renamed from drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled 2005-11-04 18:26:59.000000000 -0800 +6 -0
+       cardcount = eisa_driver_register(&dgrs_eisa_driver);
+       if (cardcount < 0)
+               return cardcount;
 #endif
+       cardcount = pci_register_driver(&dgrs_pci_driver);
+       if (cardcount)
+               return cardcount;
        return 0;
 }
 

Signed-off-by Ashutosh Naik <[email protected]>

Acked. The above patch does the trick too. Any one can be committed.

Cheers
Ashutosh

drivers/net/dgrs.c +6 -2

diff -Naurp linux-2.6.14/drivers/net/dgrs.c linux-2.6.14-git1/drivers/net/dgrs.c
@@ -1549,8 +1549,12 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
 static int __init dgrs_init_module (void)
 {
 	int	i;
-	int eisacount = 0, pcicount = 0;
-
+#ifdef CONFIG_EISA
+	int eisacount = 0;
+#endif
+#ifdef CONFIG_PCI
+	int pcicount = 0;
+#endif
 	/*
 	 *	Command line variable overrides
 	 *		debug=NNN

Richard Knutsson

2005-11-05 08:32 UTC #9 lore raw

In reply to Andrew Morton

Quoted 27 lines · Andrew Morton

In fact we can de-ifdef things a bit.

diff -puN drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled drivers/net/dgrs.c

drivers/net/dgrs.c renamed from drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled +1 -9

@@ -1549,7 +1549,7 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
 static int __init dgrs_init_module (void)
 {
 	int	i;
-	int eisacount = 0, pcicount = 0;
+	int	cardcount = 0;
 
 	/*
 	 *	Command line variable overrides

@@ -1591,15 +1591,13 @@ static int __init dgrs_init_module (void
 	 *	Find and configure all the cards
 	 */
 #ifdef CONFIG_EISA
-	eisacount = eisa_driver_register(&dgrs_eisa_driver);
-	if (eisacount < 0)
-		return eisacount;
-#endif
-#ifdef CONFIG_PCI
-	pcicount = pci_register_driver(&dgrs_pci_driver);
-	if (pcicount)
-		return pcicount;

drivers/net/dgrs.c static int __init dgrs_init_module (void · line 1603

Quoted 10 lines

drivers/net/dgrs.c renamed from drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled +6 -0

+	cardcount = eisa_driver_register(&dgrs_eisa_driver);
+	if (cardcount < 0)
+		return cardcount;
 #endif
+	cardcount = pci_register_driver(&dgrs_pci_driver);
+	if (cardcount)
+		return cardcount;
 	return 0;
 }

I do not know what to think about this one:
* reduce one #ifdef: good
* check for something clearly stated not to: not so good

But as Ashutosh Naik said: Any one can be committed.

/Richard

Andrew Morton

2005-11-05 08:46 UTC #10 lore raw

Quoted 17 lines · Richard Knutsson

Quoted 17 lines
	 */
#ifdef CONFIG_EISA
drivers/net/dgrs.c renamed from drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled +3 -8
-	eisacount = eisa_driver_register(&dgrs_eisa_driver);
-	if (eisacount < 0)
-		return eisacount;
-#endif
-#ifdef CONFIG_PCI
-	pcicount = pci_register_driver(&dgrs_pci_driver);
-	if (pcicount)
-		return pcicount;
+	cardcount = eisa_driver_register(&dgrs_eisa_driver);
+	if (cardcount < 0)
+		return cardcount;
#endif
drivers/net/dgrs.c renamed from drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled +2 -0
+	cardcount = pci_register_driver(&dgrs_pci_driver);
+	if (cardcount)

Quoted 8 lines

Quoted 5 lines
(quoted diff) +1 -0
+		return cardcount;
	return 0;
}
I do not know what to think about this one:
* reduce one #ifdef: good
* check for something clearly stated not to: not so good

Well a nicer fix would be to provide a stub implementation of eisa_driver_register() if !CONFIG_EISA, just like pci_register_driver(). Then all the ifdefs go away and the compiler removes all the code for us, after checking that we typed it correctly.

Richard Knutsson

2005-11-05 11:49 UTC #11 lore raw

Quoted 32 lines · Andrew Morton

Quoted 18 lines · Richard Knutsson
Quoted 18 lines
	 */
#ifdef CONFIG_EISA
drivers/net/dgrs.c renamed from drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled +3 -8
-	eisacount = eisa_driver_register(&dgrs_eisa_driver);
-	if (eisacount < 0)
-		return eisacount;
-#endif
-#ifdef CONFIG_PCI
-	pcicount = pci_register_driver(&dgrs_pci_driver);
-	if (pcicount)
-		return pcicount;
+	cardcount = eisa_driver_register(&dgrs_eisa_driver);
+	if (cardcount < 0)
+		return cardcount;
#endif
drivers/net/dgrs.c renamed from drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled +2 -0
+	cardcount = pci_register_driver(&dgrs_pci_driver);
+	if (cardcount)
Quoted 10 lines
Quoted 5 lines
(quoted diff) +1 -0
+		return cardcount;
	return 0;
}
I do not know what to think about this one:
* reduce one #ifdef: good
* check for something clearly stated not to: not so good

Quoted 5 lines

Well a nicer fix would be to provide a stub implementation of
eisa_driver_register() if !CONFIG_EISA, just like pci_register_driver(). Then all the ifdefs go away and the compiler removes all the code for us, after checking that we typed it correctly.

Oh, sorry. Missed the stub implementation of the pci-driver. I "ack" your patch.

BTW, can anyone ack or is that up to the maintainers?
BTW #2, why not remove #ifdef CONFIG_PCI on dgrs_cleanup_module() at the same time? Or maybe that should be in a "remove config_pci"-patch...

/Richard

Richard Knutsson

2005-11-05 15:25 UTC #12 lore raw

Quoted 38 lines · Richard Knutsson

Quoted 31 lines · Andrew Morton
Quoted 17 lines · Richard Knutsson
Quoted 17 lines
     */
#ifdef CONFIG_EISA
drivers/net/dgrs.c renamed from drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled +3 -8
-    eisacount = eisa_driver_register(&dgrs_eisa_driver);
-    if (eisacount < 0)
-        return eisacount;
-#endif
-#ifdef CONFIG_PCI
-    pcicount = pci_register_driver(&dgrs_pci_driver);
-    if (pcicount)
-        return pcicount;
+    cardcount = eisa_driver_register(&dgrs_eisa_driver);
+    if (cardcount < 0)
+        return cardcount;
#endif
+ cardcount = pci_register_driver(&dgrs_pci_driver);
Quoted 9 lines
Quoted 5 lines
drivers/net/dgrs.c renamed from drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled +2 -0
+    if (cardcount)
+        return cardcount;
    return 0;
}
Quoted 1 line
I do not know what to think about this one:
* reduce one #ifdef: good
* check for something clearly stated not to: not so good
Quoted 5 lines
Well a nicer fix would be to provide a stub implementation of
eisa_driver_register() if !CONFIG_EISA, just like pci_register_driver(). Then all the ifdefs go away and the compiler removes all the code for us, after checking that we typed it correctly.

Quoted 6 lines

Oh, sorry. Missed the stub implementation of the pci-driver. I "ack" your patch.
BTW, can anyone ack or is that up to the maintainers?
BTW #2, why not remove #ifdef CONFIG_PCI on dgrs_cleanup_module() at the same time? Or maybe that should be in a "remove config_pci"-patch...
/Richard

Just realized; what happens if CONFIG_EISA && !CONFIG_PCI and eisa_driver_register() returns value > 0, then the if-statement for the pci-driver is going to return the value, instead of 0.

/Richard

Andrew Morton

2005-11-05 17:45 UTC #13 lore raw

Quoted 50 lines · Richard Knutsson

Quoted 47 lines · Richard Knutsson
Quoted 31 lines · Andrew Morton
Quoted 17 lines · Richard Knutsson
Quoted 17 lines
     */
#ifdef CONFIG_EISA
drivers/net/dgrs.c renamed from drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled +3 -8
-    eisacount = eisa_driver_register(&dgrs_eisa_driver);
-    if (eisacount < 0)
-        return eisacount;
-#endif
-#ifdef CONFIG_PCI
-    pcicount = pci_register_driver(&dgrs_pci_driver);
-    if (pcicount)
-        return pcicount;
+    cardcount = eisa_driver_register(&dgrs_eisa_driver);
+    if (cardcount < 0)
+        return cardcount;
#endif
+ cardcount = pci_register_driver(&dgrs_pci_driver);
Quoted 9 lines
Quoted 5 lines
drivers/net/dgrs.c renamed from drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled +2 -0
+    if (cardcount)
+        return cardcount;
    return 0;
}
Quoted 1 line
I do not know what to think about this one:
* reduce one #ifdef: good
* check for something clearly stated not to: not so good
Quoted 7 lines
Well a nicer fix would be to provide a stub implementation of eisa_driver_register() if !CONFIG_EISA, just like pci_register_driver(). Then all the ifdefs go away and the compiler removes all the code for us,
after checking that we typed it correctly.
Oh, sorry. Missed the stub implementation of the pci-driver. I "ack" your patch.
BTW, can anyone ack or is that up to the maintainers?
BTW #2, why not remove #ifdef CONFIG_PCI on dgrs_cleanup_module() at the same time? Or maybe that should be in a "remove config_pci"-patch...

Quoted 5 lines

Quoted 1 line
/Richard
Just realized; what happens if CONFIG_EISA && !CONFIG_PCI and eisa_driver_register() returns value > 0, then the if-statement for the pci-driver is going to return the value, instead of 0.

if !CONFIG_PCI, pci_register_driver() will return zero.

Andrew Morton

2005-11-05 17:46 UTC #14 lore raw

In reply to Richard Knutsson

Quoted 2 lines · Richard Knutsson

BTW, can anyone ack or is that up to the maintainers?

It's useful info - it shows that someone else took the time to revie the code.

Quoted 2 lines

BTW #2, why not remove #ifdef CONFIG_PCI on dgrs_cleanup_module() at the same time? Or maybe that should be in a "remove config_pci"-patch...

yup. There are lots of opportunities for that, I bet.