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
Ashutosh Naik wrote:
>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
>-
>
>
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
--- 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;
}
On 11/2/05, Richard Knutsson <[email protected]> wrote:
> Ashutosh Naik wrote:
>
> >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
> >-
> >
> >
> 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
> --- 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
>
>
>>>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
>>>-
>>>
>>>
>>Signed-off-by: Richard Knutsson <[email protected]>
>>
>>---
>>
>>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
[email protected], since that is the address in the MAINTAINERS-file.
DS
On 11/3/05, Richard Knutsson <[email protected]> wrote:
> >
> >
> >>>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
> >>>-
> >>>
> >>>
>
> >>Signed-off-by: Richard Knutsson <[email protected]>
> >>
> >>---
> >>
> >>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
>>>>>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
>>>>>
>>>>>
>
>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 <[email protected]>
---
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 <[email protected]> wrote:
>
> >
> >
> >>>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
> >>>-
> >>>
> >>>
>
> >>Signed-off-by: Richard Knutsson <[email protected]>
> >>
> >>---
> >>
> >>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 <[email protected]> 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 <[email protected]> wrote:
> Andrew Morton <[email protected]> wrote:
> >
> > 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]>
> 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;
> }
>
Signed-off-by: Ashutosh Naik <[email protected]>
Acked. The above patch does the trick too. Any one can be committed.
Cheers
Ashutosh
Andrew Morton wrote:
>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;
> }
>
>
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 <[email protected]> wrote:
>
> > */
> > #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 <[email protected]> wrote:
>
>
>>> */
>>>
>>>
>> > #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:
>
>> Richard Knutsson <[email protected]> wrote:
>>
>>
>>>> */
>>>>
>>>
>>> > #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 <[email protected]> wrote:
>
> Richard Knutsson wrote:
>
> > Andrew Morton wrote:
> >
> >> Richard Knutsson <[email protected]> wrote:
> >>
> >>
> >>>> */
> >>>>
> >>>
> >>> > #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 <[email protected]> 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.