Re: [PATCH]dgrs - Fixes Warnings when CONFIG_ISA and CONFIG_PCI are not enabled

quoted

quoted

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

Signed-off-by: Ashutosh Naik <redacted>

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

quoted

Signed-off-by: Richard Knutsson <redacted>

---

diff -uNr a/drivers/net/dgrs.c b/drivers/net/dgrs.c

--- a/drivers/net/dgrs.c        2005-08-29 01:41:01.000000000 +0200
+++ b/drivers/net/dgrs.c        2005-10-26 15:53:43.000000000 +0200

@@ -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;
}
   

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

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 
netdev@vger.kernel.org, since that is the address in the MAINTAINERS-file.
DS

On 11/3/05, Richard Knutsson [off-list ref] wrote:

quoted

quoted

quoted

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

Signed-off-by: Ashutosh Naik <redacted>

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

quoted

quoted

Signed-off-by: Richard Knutsson <redacted>

---

diff -uNr a/drivers/net/dgrs.c b/drivers/net/dgrs.c

--- a/drivers/net/dgrs.c        2005-08-29 01:41:01.000000000 +0200
+++ b/drivers/net/dgrs.c        2005-10-26 15:53:43.000000000 +0200

@@ -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;
}

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

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.

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

quoted

quoted

quoted

quoted

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

Signed-off-by: Ashutosh Naik <redacted>	

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

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
 

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 <redacted>

---

diff -Nurp a/drivers/net/dgrs.c b/drivers/net/dgrs.c

--- a/drivers/net/dgrs.c	2005-10-28 02:02:08.000000000 +0200
+++ b/drivers/net/dgrs.c	2005-11-02 10:19:43.000000000 +0100

@@ -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
 	 */
 #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;
 }

Richard Knutsson [off-list ref] wrote:

quoted

quoted

quoted

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

Signed-off-by: Ashutosh Naik <redacted>

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

quoted

quoted

Signed-off-by: Richard Knutsson <redacted>

---

diff -uNr a/drivers/net/dgrs.c b/drivers/net/dgrs.c

--- a/drivers/net/dgrs.c        2005-08-29 01:41:01.000000000 +0200
+++ b/drivers/net/dgrs.c        2005-10-26 15:53:43.000000000 +0200

@@ -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;
}
   

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

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)

I think you patch is better.

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

Andrew Morton [off-list ref] wrote:

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.

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

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

@@ -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;
 }
 

_

Hi Andrew,

On 11/5/05, Andrew Morton [off-list ref] wrote:

Andrew Morton [off-list ref] wrote:

quoted

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 <redacted>

Signed-off-by: Ashutosh Naik <redacted>

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

Cheers
Ashutosh

Andrew Morton wrote:

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

Richard Knutsson [off-list ref] wrote:

quoted

	 */

 > #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;
 > }
 >  
 >
 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.

Andrew Morton wrote:

Richard Knutsson [off-list ref] wrote:
 

quoted

quoted

 */
     

#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;
}
 

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.
 

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 wrote:

Andrew Morton wrote:

quoted

Richard Knutsson [off-list ref] wrote:
 

quoted

quoted

     */
    

quoted

#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;
}
 >

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.
 

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

Richard Knutsson [off-list ref] wrote:

Richard Knutsson wrote:

quoted

Andrew Morton wrote:

quoted

Richard Knutsson [off-list ref] wrote:
 

quoted

quoted

     */
    

quoted

#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;
}
 >

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.
 

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.

if !CONFIG_PCI, pci_register_driver() will return zero.

Richard Knutsson [off-list ref] wrote:

 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.

 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.