* 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).