* Re: [PATCH -mm2] net: Fix compiler-error on dgrs.c when !CONFIG_PCI
[not found] <20051120014408.20407.66374.sendpatchset@thinktank.campus.ltu.se>
@ 2005-11-20 10:24 ` Herbert Xu
2005-11-20 15:35 ` Richard Knutsson
0 siblings, 1 reply; 10+ messages in thread
From: Herbert Xu @ 2005-11-20 10:24 UTC (permalink / raw)
To: Richard Knutsson
Cc: akpm, linux-kernel, ricknu-0, netdev, jgarzik, ashutosh.naik
Richard Knutsson <ricknu-0@student.ltu.se> wrote:
>
> diff -Narup a/drivers/net/dgrs.c b/drivers/net/dgrs.c
> --- a/drivers/net/dgrs.c 2005-11-19 20:17:51.000000000 +0100
> +++ b/drivers/net/dgrs.c 2005-11-19 20:29:52.000000000 +0100
> @@ -1458,6 +1458,8 @@ static struct pci_driver dgrs_pci_driver
> .probe = dgrs_pci_probe,
> .remove = __devexit_p(dgrs_pci_remove),
> };
> +#else
> +static struct pci_driver dgrs_pci_driver = {};
> #endif
How about something like this?
[NETDRV] dgrs: Fix compiler-error on dgrs.c when !CONFIG_PCI
The previous patch on this driver removed the #ifdef CONFIG_PCI
around the pci_register_driver call in order to avoid a warning
about an unused variable. This produces an error since the PCI
driver object is undefined if CONFIG_PCI is not set.
Instead of doing that, we should simply make sure that the
cardcount variable is always used.
The following patch does this by making inline function wrappers
to do the actual registration/deregistration.
This problem was reported by Richard Knutsson.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/drivers/net/dgrs.c b/drivers/net/dgrs.c
--- a/drivers/net/dgrs.c
+++ b/drivers/net/dgrs.c
@@ -1458,6 +1458,25 @@ static struct pci_driver dgrs_pci_driver
.probe = dgrs_pci_probe,
.remove = __devexit_p(dgrs_pci_remove),
};
+
+static inline int dgrs_register_pci(void)
+{
+ return pci_register_driver(&dgrs_pci_driver);
+}
+
+static inline void dgrs_unregister_pci(void)
+{
+ pci_unregister_driver(&dgrs_pci_driver);
+}
+#else
+static inline int dgrs_register_pci(void)
+{
+ return 0;
+}
+
+static inline int dgrs_unregister_pci(void)
+{
+}
#endif
@@ -1520,6 +1539,25 @@ static struct eisa_driver dgrs_eisa_driv
.remove = __devexit_p(dgrs_eisa_remove),
}
};
+
+static inline int dgrs_register_eisa(void)
+{
+ return eisa_driver_register(&dgrs_eisa_driver);
+}
+
+static inline void dgrs_unregister_eisa(void)
+{
+ eisa_driver_unregister(&dgrs_eisa_driver);
+}
+#else
+static inline int dgrs_register_eisa(void)
+{
+ return 0;
+}
+
+static inline void dgrs_unregister_eisa(void)
+{
+}
#endif
/*
@@ -1590,25 +1628,21 @@ static int __init dgrs_init_module (void
/*
* Find and configure all the cards
*/
-#ifdef CONFIG_EISA
- cardcount = eisa_driver_register(&dgrs_eisa_driver);
+ cardcount = dgrs_register_eisa();
if (cardcount < 0)
return cardcount;
-#endif
- cardcount = pci_register_driver(&dgrs_pci_driver);
- if (cardcount)
+ cardcount = dgrs_register_pci();
+ if (cardcount < 0) {
+ dgrs_unregister_eisa();
return cardcount;
+ }
return 0;
}
static void __exit dgrs_cleanup_module (void)
{
-#ifdef CONFIG_EISA
- eisa_driver_unregister (&dgrs_eisa_driver);
-#endif
-#ifdef CONFIG_PCI
- pci_unregister_driver (&dgrs_pci_driver);
-#endif
+ dgrs_unregister_pci();
+ dgrs_unregister_eisa();
}
module_init(dgrs_init_module);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -mm2] net: Fix compiler-error on dgrs.c when !CONFIG_PCI
2005-11-20 10:24 ` [PATCH -mm2] net: Fix compiler-error on dgrs.c when !CONFIG_PCI Herbert Xu
@ 2005-11-20 15:35 ` Richard Knutsson
2005-11-20 20:40 ` Herbert Xu
0 siblings, 1 reply; 10+ messages in thread
From: Richard Knutsson @ 2005-11-20 15:35 UTC (permalink / raw)
To: Herbert Xu; +Cc: akpm, linux-kernel, netdev, jgarzik, ashutosh.naik
Herbert Xu wrote:
>Richard Knutsson <ricknu-0@student.ltu.se> wrote:
>
>
>>diff -Narup a/drivers/net/dgrs.c b/drivers/net/dgrs.c
>>--- a/drivers/net/dgrs.c 2005-11-19 20:17:51.000000000 +0100
>>+++ b/drivers/net/dgrs.c 2005-11-19 20:29:52.000000000 +0100
>>@@ -1458,6 +1458,8 @@ static struct pci_driver dgrs_pci_driver
>> .probe = dgrs_pci_probe,
>> .remove = __devexit_p(dgrs_pci_remove),
>>};
>>+#else
>>+static struct pci_driver dgrs_pci_driver = {};
>>#endif
>>
>>
>
>How about something like this?
>
>[NETDRV] dgrs: Fix compiler-error on dgrs.c when !CONFIG_PCI
>
>
[NETDRV], is that "standard" formating? If so, is there any more? Thanks :)
>The previous patch on this driver removed the #ifdef CONFIG_PCI
>around the pci_register_driver call in order to avoid a warning
>about an unused variable. This produces an error since the PCI
>driver object is undefined if CONFIG_PCI is not set.
>
>
Well, not really. The removal of CONFIG_PCI was just a final touch by
Andrew Morton. The only real change were from having two variables, for
eisa and pci, to having one common.
>Instead of doing that, we should simply make sure that the
>cardcount variable is always used.
>
>
Is it not? The problem were that dgrs_pci_driver was not defined if
!CONFIG_PCI, and because the pci_*-functions have empty shells if
!CONFIG_PCI, it is quite nice to use them without #ifdef.
So the simplest solution (as I saw it) was to define an empty
dgrs_pci_driver when CONFIG_PCI is not set.
On the design perspective, I would argue that struct's is more
appropriate within #ifdef's then functions.
>The following patch does this by making inline function wrappers
>to do the actual registration/deregistration.
>
>
I must say, I like that eisa gets addressed in the same manner as pci.
But is it not more appropriate to implement empty eisa-functions if
!CONFIG_EISA in linux/eisa.h (or equal)?
>This problem was reported by Richard Knutsson.
>
>Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
>Cheers,
>
>
Thank you for the review,
Richard Knutsson
PS
You realize you have not tushed the cardcount-"problem"? ;)
DS
> diff --git a/drivers/net/dgrs.c b/drivers/net/dgrs.c
> --- a/drivers/net/dgrs.c
> +++ b/drivers/net/dgrs.c
> @@ -1458,6 +1458,25 @@ static struct pci_driver dgrs_pci_driver
> .probe = dgrs_pci_probe,
> .remove = __devexit_p(dgrs_pci_remove),
> };
> +
> +static inline int dgrs_register_pci(void)
> +{
> + return pci_register_driver(&dgrs_pci_driver);
> +}
> +
> +static inline void dgrs_unregister_pci(void)
> +{
> + pci_unregister_driver(&dgrs_pci_driver);
> +}
> +#else
> +static inline int dgrs_register_pci(void)
> +{
> + return 0;
> +}
> +
> +static inline int dgrs_unregister_pci(void)
> +{
> +}
> #endif
>
>
> @@ -1520,6 +1539,25 @@ static struct eisa_driver dgrs_eisa_driv
> .remove = __devexit_p(dgrs_eisa_remove),
> }
> };
> +
> +static inline int dgrs_register_eisa(void)
> +{
> + return eisa_driver_register(&dgrs_eisa_driver);
> +}
> +
> +static inline void dgrs_unregister_eisa(void)
> +{
> + eisa_driver_unregister(&dgrs_eisa_driver);
> +}
> +#else
> +static inline int dgrs_register_eisa(void)
> +{
> + return 0;
> +}
> +
> +static inline void dgrs_unregister_eisa(void)
> +{
> +}
> #endif
>
> /*
> @@ -1590,25 +1628,21 @@ static int __init dgrs_init_module (void
> /*
> * Find and configure all the cards
> */
> -#ifdef CONFIG_EISA
> - cardcount = eisa_driver_register(&dgrs_eisa_driver);
> + cardcount = dgrs_register_eisa();
> if (cardcount < 0)
> return cardcount;
> -#endif
> - cardcount = pci_register_driver(&dgrs_pci_driver);
> - if (cardcount)
> + cardcount = dgrs_register_pci();
> + if (cardcount < 0) {
Are you sure it should be "cardcount < 0" and not "cardcount"?
> + dgrs_unregister_eisa();
Why change the behaviour off this driver?
> return cardcount;
> + }
> return 0;
> }
>
> static void __exit dgrs_cleanup_module (void)
> {
> -#ifdef CONFIG_EISA
> - eisa_driver_unregister (&dgrs_eisa_driver);
> -#endif
> -#ifdef CONFIG_PCI
> - pci_unregister_driver (&dgrs_pci_driver);
> -#endif
> + dgrs_unregister_pci();
> + dgrs_unregister_eisa();
> }
>
> module_init(dgrs_init_module);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -mm2] net: Fix compiler-error on dgrs.c when !CONFIG_PCI
2005-11-20 15:35 ` Richard Knutsson
@ 2005-11-20 20:40 ` Herbert Xu
2005-11-21 12:52 ` Richard Knutsson
0 siblings, 1 reply; 10+ messages in thread
From: Herbert Xu @ 2005-11-20 20:40 UTC (permalink / raw)
To: Richard Knutsson; +Cc: akpm, linux-kernel, netdev, jgarzik, ashutosh.naik
On Sun, Nov 20, 2005 at 04:35:46PM +0100, Richard Knutsson wrote:
>
> >-#ifdef CONFIG_EISA
> >- cardcount = eisa_driver_register(&dgrs_eisa_driver);
> >+ cardcount = dgrs_register_eisa();
> > if (cardcount < 0)
> > return cardcount;
> >-#endif
> >- cardcount = pci_register_driver(&dgrs_pci_driver);
> >- if (cardcount)
> >+ cardcount = dgrs_register_pci();
> >+ if (cardcount < 0) {
> Are you sure it should be "cardcount < 0" and not "cardcount"?
Yes if cardcount is >= 0 then the registration was successful.
> >+ dgrs_unregister_eisa();
> Why change the behaviour off this driver?
Because the driver was buggy. When this function returns a non-zero
value, it must return the system to its original state.
That means if the EISA driver has already been registered then it must
be unregistered.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -mm2] net: Fix compiler-error on dgrs.c when !CONFIG_PCI
2005-11-20 20:40 ` Herbert Xu
@ 2005-11-21 12:52 ` Richard Knutsson
2005-11-21 20:37 ` Herbert Xu
0 siblings, 1 reply; 10+ messages in thread
From: Richard Knutsson @ 2005-11-21 12:52 UTC (permalink / raw)
To: Herbert Xu; +Cc: akpm, linux-kernel, netdev, jgarzik, ashutosh.naik
Herbert Xu wrote:
>On Sun, Nov 20, 2005 at 04:35:46PM +0100, Richard Knutsson wrote:
>
>
>>>-#ifdef CONFIG_EISA
>>>- cardcount = eisa_driver_register(&dgrs_eisa_driver);
>>>+ cardcount = dgrs_register_eisa();
>>> if (cardcount < 0)
>>> return cardcount;
>>>-#endif
>>>- cardcount = pci_register_driver(&dgrs_pci_driver);
>>>- if (cardcount)
>>>+ cardcount = dgrs_register_pci();
>>>+ if (cardcount < 0) {
>>>
>>>
>>Are you sure it should be "cardcount < 0" and not "cardcount"?
>>
>>
>
>Yes if cardcount is >= 0 then the registration was successful.
>
>
>
>>>+ dgrs_unregister_eisa();
>>>
>>>
>>Why change the behaviour off this driver?
>>
>>
>
>Because the driver was buggy. When this function returns a non-zero
>value, it must return the system to its original state.
>
>That means if the EISA driver has already been registered then it must
>be unregistered.
>
>Cheers,
>
>
What do you think about this patch? Will you sign it? It is no longer an
error-warning-fix but a bug-fix (and some cleanup).
I "took" you implementation of dgrs_(un)register_eisa(), especially
since eisa needed to be unregistered if pci succeeds (I take you word
for it to be so).
(BTW, this patch is compiled with CONFIG_PCI set, CONFIG_EISA set and
both set without errors/warnings for dgrs.o.)
This patch requirer the
"net-fix-compiler-error-on-dgrsc-when-config_pci.patch" (added to the
-mm tree after 2.6.15-rc1-mm2):
--- devel/drivers/net/dgrs.c~net-fix-compiler-error-on-dgrsc-when-config_pci 2005-11-19 18:00:34.000000000 -0800
+++ devel-akpm/drivers/net/dgrs.c 2005-11-19 18:00:34.000000000 -0800
@@ -1458,6 +1458,8 @@ static struct pci_driver dgrs_pci_driver
.probe = dgrs_pci_probe,
.remove = __devexit_p(dgrs_pci_remove),
};
+#else
+static struct pci_driver dgrs_pci_driver = {};
#endif
Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
---
diff -Narup a/drivers/net/dgrs.c b/drivers/net/dgrs.c
--- a/drivers/net/dgrs.c 2005-11-21 11:25:29.000000000 +0100
+++ b/drivers/net/dgrs.c 2005-11-21 11:38:33.000000000 +0100
@@ -1522,6 +1522,26 @@ static struct eisa_driver dgrs_eisa_driv
.remove = __devexit_p(dgrs_eisa_remove),
}
};
+
+
+static inline int dgrs_register_eisa(void)
+{
+ return eisa_driver_register(&dgrs_eisa_driver);
+}
+
+static inline void dgrs_unregister_eisa(void)
+{
+ eisa_driver_unregister(&dgrs_eisa_driver);
+}
+
+#else
+
+static inline int dgrs_register_eisa(void)
+{
+ return 0;
+}
+
+static inline void dgrs_unregister_eisa(void) {}
#endif
/*
@@ -1551,7 +1571,7 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
static int __init dgrs_init_module (void)
{
int i;
- int cardcount = 0;
+ int cardcount;
/*
* Command line variable overrides
@@ -1592,25 +1612,23 @@ static int __init dgrs_init_module (void
/*
* Find and configure all the cards
*/
-#ifdef CONFIG_EISA
- cardcount = eisa_driver_register(&dgrs_eisa_driver);
+
+ cardcount = dgrs_register_eisa();
if (cardcount < 0)
return cardcount;
-#endif
+
cardcount = pci_register_driver(&dgrs_pci_driver);
- if (cardcount)
+ if (cardcount < 0) {
+ dgrs_unregister_eisa();
return cardcount;
+ }
return 0;
}
static void __exit dgrs_cleanup_module (void)
{
-#ifdef CONFIG_EISA
- eisa_driver_unregister (&dgrs_eisa_driver);
-#endif
-#ifdef CONFIG_PCI
+ dgrs_unregister_eisa();
pci_unregister_driver (&dgrs_pci_driver);
-#endif
}
module_init(dgrs_init_module);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -mm2] net: Fix compiler-error on dgrs.c when !CONFIG_PCI
2005-11-21 12:52 ` Richard Knutsson
@ 2005-11-21 20:37 ` Herbert Xu
2005-11-21 21:47 ` Richard Knutsson
2005-11-22 1:56 ` Richard Knutsson
0 siblings, 2 replies; 10+ messages in thread
From: Herbert Xu @ 2005-11-21 20:37 UTC (permalink / raw)
To: Richard Knutsson; +Cc: akpm, linux-kernel, netdev, jgarzik, ashutosh.naik
On Mon, Nov 21, 2005 at 01:52:49PM +0100, Richard Knutsson wrote:
>
> What do you think about this patch? Will you sign it? It is no longer an
> error-warning-fix but a bug-fix (and some cleanup).
> I "took" you implementation of dgrs_(un)register_eisa(), especially
> since eisa needed to be unregistered if pci succeeds (I take you word
> for it to be so).
> (BTW, this patch is compiled with CONFIG_PCI set, CONFIG_EISA set and
> both set without errors/warnings for dgrs.o.)
>
> This patch requirer the
> "net-fix-compiler-error-on-dgrsc-when-config_pci.patch" (added to the
> -mm tree after 2.6.15-rc1-mm2):
>
> ---
> devel/drivers/net/dgrs.c~net-fix-compiler-error-on-dgrsc-when-config_pci
> 2005-11-19 18:00:34.000000000 -0800
> +++ devel-akpm/drivers/net/dgrs.c 2005-11-19 18:00:34.000000000 -0800
> @@ -1458,6 +1458,8 @@ static struct pci_driver dgrs_pci_driver
> .probe = dgrs_pci_probe,
> .remove = __devexit_p(dgrs_pci_remove),
> };
> +#else
> +static struct pci_driver dgrs_pci_driver = {};
> #endif
I don't see the point. We shouldn't have this structure at all
if CONFIG_PCI is not set.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -mm2] net: Fix compiler-error on dgrs.c when !CONFIG_PCI
2005-11-21 20:37 ` Herbert Xu
@ 2005-11-21 21:47 ` Richard Knutsson
2005-11-21 22:12 ` Herbert Xu
2005-11-22 1:56 ` Richard Knutsson
1 sibling, 1 reply; 10+ messages in thread
From: Richard Knutsson @ 2005-11-21 21:47 UTC (permalink / raw)
To: Herbert Xu; +Cc: akpm, linux-kernel, netdev, jgarzik, ashutosh.naik
Herbert Xu wrote:
>On Mon, Nov 21, 2005 at 01:52:49PM +0100, Richard Knutsson wrote:
>
>
>>This patch requirer the
>>"net-fix-compiler-error-on-dgrsc-when-config_pci.patch" (added to the
>>-mm tree after 2.6.15-rc1-mm2):
>>
>>---
>>devel/drivers/net/dgrs.c~net-fix-compiler-error-on-dgrsc-when-config_pci
>>2005-11-19 18:00:34.000000000 -0800
>>+++ devel-akpm/drivers/net/dgrs.c 2005-11-19 18:00:34.000000000 -0800
>>@@ -1458,6 +1458,8 @@ static struct pci_driver dgrs_pci_driver
>> .probe = dgrs_pci_probe,
>> .remove = __devexit_p(dgrs_pci_remove),
>>};
>>+#else
>>+static struct pci_driver dgrs_pci_driver = {};
>>#endif
>>
>>
>
>I don't see the point. We shouldn't have this structure at all
>if CONFIG_PCI is not set.
>
>Cheers,
>
We need it even if pci_register_driver() and pci_register_driver() is
empty shells. And instead of removing #endif CONFIG_PCI for
dgrs_pci_driver, which needs dgrs_pci_tbl, dgrs_pci_probe() and
dgrs_pci_remove() (and so on), I added the empty dgrs_pci_driver-structure.
cu,
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -mm2] net: Fix compiler-error on dgrs.c when !CONFIG_PCI
2005-11-21 21:47 ` Richard Knutsson
@ 2005-11-21 22:12 ` Herbert Xu
2005-11-21 23:20 ` Richard Knutsson
0 siblings, 1 reply; 10+ messages in thread
From: Herbert Xu @ 2005-11-21 22:12 UTC (permalink / raw)
To: Richard Knutsson
Cc: herbert, akpm, linux-kernel, netdev, jgarzik, ashutosh.naik
Richard Knutsson <ricknu-0@student.ltu.se> wrote:
>
> We need it even if pci_register_driver() and pci_register_driver() is
> empty shells. And instead of removing #endif CONFIG_PCI for
I don't think so. Please see my previous patch where pci_register_driver
is not called at all if CONFIG_PCI is not defined.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -mm2] net: Fix compiler-error on dgrs.c when !CONFIG_PCI
2005-11-21 22:12 ` Herbert Xu
@ 2005-11-21 23:20 ` Richard Knutsson
2005-11-21 23:36 ` Herbert Xu
0 siblings, 1 reply; 10+ messages in thread
From: Richard Knutsson @ 2005-11-21 23:20 UTC (permalink / raw)
To: Herbert Xu; +Cc: akpm, linux-kernel, netdev, jgarzik, ashutosh.naik
Herbert Xu wrote:
>Richard Knutsson <ricknu-0@student.ltu.se> wrote:
>
>
>>We need it even if pci_register_driver() and pci_register_driver() is
>>empty shells. And instead of removing #endif CONFIG_PCI for
>>
>>
>
>I don't think so. Please see my previous patch where pci_register_driver
>is not called at all if CONFIG_PCI is not defined.
>
>Cheers,
>
>
Yes, I know you patch don't need it (thought you commented the patch vs.
dgrs.c, not vs. you patch). But to do that, you have to introduce a new
dgrs-specific pci-layer to compensate. If you don't want to have an
empty struct, is it not nicer/easier to just #ifdef CONFIG_PCI the
pci_*-functions? (instead of two inline functions for every used
pci_*-function?)
Am thinking of removing the #ifdef CONFIG_PCI's in other files, to
"clean up" the code, but that would introduce this problem again, don't
you think it is more readable to make an empty struct when !CONFIG_PCI,
then making new pci_*-functions specific to the driver?
cu
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -mm2] net: Fix compiler-error on dgrs.c when !CONFIG_PCI
2005-11-21 23:20 ` Richard Knutsson
@ 2005-11-21 23:36 ` Herbert Xu
0 siblings, 0 replies; 10+ messages in thread
From: Herbert Xu @ 2005-11-21 23:36 UTC (permalink / raw)
To: Richard Knutsson; +Cc: akpm, linux-kernel, netdev, jgarzik, ashutosh.naik
On Tue, Nov 22, 2005 at 12:20:30AM +0100, Richard Knutsson wrote:
>
> Am thinking of removing the #ifdef CONFIG_PCI's in other files, to
> "clean up" the code, but that would introduce this problem again, don't
> you think it is more readable to make an empty struct when !CONFIG_PCI,
> then making new pci_*-functions specific to the driver?
I think my version is more readable. However, it's really up to Jeff
to decide.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -mm2] net: Fix compiler-error on dgrs.c when !CONFIG_PCI
2005-11-21 20:37 ` Herbert Xu
2005-11-21 21:47 ` Richard Knutsson
@ 2005-11-22 1:56 ` Richard Knutsson
1 sibling, 0 replies; 10+ messages in thread
From: Richard Knutsson @ 2005-11-22 1:56 UTC (permalink / raw)
To: Herbert Xu; +Cc: akpm, linux-kernel, netdev, jgarzik, ashutosh.naik
Herbert Xu wrote:
>On Mon, Nov 21, 2005 at 01:52:49PM +0100, Richard Knutsson wrote:
>
>
>>This patch requirer the
>>"net-fix-compiler-error-on-dgrsc-when-config_pci.patch" (added to the
>>-mm tree after 2.6.15-rc1-mm2):
>>
>>---
>>devel/drivers/net/dgrs.c~net-fix-compiler-error-on-dgrsc-when-config_pci
>>2005-11-19 18:00:34.000000000 -0800
>>+++ devel-akpm/drivers/net/dgrs.c 2005-11-19 18:00:34.000000000 -0800
>>@@ -1458,6 +1458,8 @@ static struct pci_driver dgrs_pci_driver
>> .probe = dgrs_pci_probe,
>> .remove = __devexit_p(dgrs_pci_remove),
>>};
>>+#else
>>+static struct pci_driver dgrs_pci_driver = {};
>>#endif
>>
>>
>
>I don't see the point. We shouldn't have this structure at all
>if CONFIG_PCI is not set.
>
>Cheers,
>
>
Opps, misread your mail. Sorry.
But in that case, why shall we have any pci_*-function in the first
place when !CONFIG_PCI? As it was before, they were contained within
#ifdef CONFIG_PCI's.
You said your patch were easier to read, please elaborate. Yes, in the
dgrs_init_module() (no arguments there), but you introduces new
functions (who need to be checked out if you read the code for the first
time) and is really #ifdef's a good idea to change function behavior?
Isn't better to change the input? (I know, linux/pci.h does it, but at
least that is in a .h-file with inline functions containing at most
"return 0;").
Btw, you said Jeff should decide. Why not Rich Richardson who is the author?
Apologize for the ranting.
As I said before, thinking of deleting CONFIG_PCI's containing
pci_*-functions and if so, need a valid plan for it (because of the
pci_driver-struct). If there is any no-no in "my" way, please point it
out to spare me/you/lkml the patches. If not for this, I would've let
this rest and leave it to Jeff (or Rich ;) ).
cu,
Richard
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2005-11-22 1:56 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20051120014408.20407.66374.sendpatchset@thinktank.campus.ltu.se>
2005-11-20 10:24 ` [PATCH -mm2] net: Fix compiler-error on dgrs.c when !CONFIG_PCI Herbert Xu
2005-11-20 15:35 ` Richard Knutsson
2005-11-20 20:40 ` Herbert Xu
2005-11-21 12:52 ` Richard Knutsson
2005-11-21 20:37 ` Herbert Xu
2005-11-21 21:47 ` Richard Knutsson
2005-11-21 22:12 ` Herbert Xu
2005-11-21 23:20 ` Richard Knutsson
2005-11-21 23:36 ` Herbert Xu
2005-11-22 1:56 ` Richard Knutsson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).