| Message ID | 20200801205242.GA9549@mx-linux-amd |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | 8390: core cleanups | expand |
On Sat, 1 Aug 2020 22:52:42 +0200 Armin Wolf wrote: > Replace version string with MODULE_* macros. > > Include necessary libraries. > > Fix two minor coding-style issues. > > Signed-off-by: Armin Wolf <W_Armin@gmx.de> This doesn't build but also ../drivers/net/ethernet/8390/lib8390.c:973:17: error: undefined identifier 'version' In file included from ../include/linux/kernel.h:15, from ../drivers/net/ethernet/8390/8390.c:9: ../drivers/net/ethernet/8390/lib8390.c: In function ‘ethdev_setup’: ../drivers/net/ethernet/8390/lib8390.c:973:17: error: ‘version’ undeclared (first use in this function) 973 | pr_info("%s", version); | ^~~~~~~ ../include/linux/printk.h:368:34: note: in definition of macro ‘pr_info’ 368 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__) | ^~~~~~~~~~~ ../drivers/net/ethernet/8390/lib8390.c:973:17: note: each undeclared identifier is reported only once for each function it appears in 973 | pr_info("%s", version); | ^~~~~~~ ../include/linux/printk.h:368:34: note: in definition of macro ‘pr_info’ 368 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__) | ^~~~~~~~~~~ make[5]: *** [../scripts/Makefile.build:281: drivers/net/ethernet/8390/8390.o] Error 1 make[4]: *** [../scripts/Makefile.build:497: drivers/net/ethernet/8390] Error 2 make[4]: *** Waiting for unfinished jobs.... make[3]: *** [../scripts/Makefile.build:497: drivers/net/ethernet] Error 2 make[3]: *** Waiting for unfinished jobs.... make[2]: *** [../scripts/Makefile.build:497: drivers/net] Error 2 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/netdev/net-next/Makefile:1771: drivers] Error 2 make: *** [Makefile:185: __sub-make] Error 2 > diff --git a/drivers/net/ethernet/8390/8390.c b/drivers/net/ethernet/8390/8390.c > index 0e0aa4016858..aabb637c1fbf 100644 > --- a/drivers/net/ethernet/8390/8390.c > +++ b/drivers/net/ethernet/8390/8390.c > @@ -1,11 +1,26 @@ > // SPDX-License-Identifier: GPL-2.0-only > -/* 8390 core for usual drivers */ > > -static const char version[] = > - "8390.c:v1.10cvs 9/23/94 Donald Becker (becker@cesdis.gsfc.nasa.gov)\n"; > +#define DRV_NAME "8390" > +#define DRV_DESCRIPTION "8390 core for usual drivers" > +#define DRV_AUTHOR "Donald Becker (becker@cesdis.gsfc.nasa.gov)" > +#define DRV_VERSION "1.10cvs" > +#define DRV_RELDATE "9/23/1994" > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/export.h> > + > +#include <linux/netdevice.h> > +#include <linux/etherdevice.h> > > #include "lib8390.c" > > +MODULE_AUTHOR(DRV_AUTHOR); > +MODULE_DESCRIPTION(DRV_DESCRIPTION); > +MODULE_VERSION(DRV_VERSION); Please drop the driver version, it looks pretty meaningless and we are moving away from driver versions in general. > +MODULE_LICENSE("GPL"); Why move this up? It's common to have these at the end. > + > int ei_open(struct net_device *dev) > { > return __ei_open(dev); > @@ -64,7 +79,7 @@ const struct net_device_ops ei_netdev_ops = { > .ndo_get_stats = ei_get_stats, > .ndo_set_rx_mode = ei_set_multicast_list, > .ndo_validate_addr = eth_validate_addr, > - .ndo_set_mac_address = eth_mac_addr, > + .ndo_set_mac_address = eth_mac_addr, > #ifdef CONFIG_NET_POLL_CONTROLLER > .ndo_poll_controller = ei_poll, > #endif > @@ -74,6 +89,7 @@ EXPORT_SYMBOL(ei_netdev_ops); > struct net_device *__alloc_ei_netdev(int size) > { > struct net_device *dev = ____alloc_ei_netdev(size); > + > if (dev) > dev->netdev_ops = &ei_netdev_ops; > return dev; > @@ -100,4 +116,3 @@ static void __exit ns8390_module_exit(void) > module_init(ns8390_module_init); > module_exit(ns8390_module_exit); > #endif /* MODULE */ > -MODULE_LICENSE("GPL"); > -- > 2.20.1
On Sat, Aug 01, 2020 at 09:32:04PM -0700, Jakub Kicinski wrote: > On Sat, 1 Aug 2020 22:52:42 +0200 Armin Wolf wrote: > > Replace version string with MODULE_* macros. > > > > Include necessary libraries. > > > > Fix two minor coding-style issues. > > > > Signed-off-by: Armin Wolf <W_Armin@gmx.de> > > This doesn't build but also > > ../drivers/net/ethernet/8390/lib8390.c:973:17: error: undefined identifier 'version' > In file included from ../include/linux/kernel.h:15, > from ../drivers/net/ethernet/8390/8390.c:9: > ../drivers/net/ethernet/8390/lib8390.c: In function ‘ethdev_setup’: > ../drivers/net/ethernet/8390/lib8390.c:973:17: error: ‘version’ undeclared (first use in this function) > 973 | pr_info("%s", version); > | ^~~~~~~ > ../include/linux/printk.h:368:34: note: in definition of macro ‘pr_info’ > 368 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__) > | ^~~~~~~~~~~ > ../drivers/net/ethernet/8390/lib8390.c:973:17: note: each undeclared identifier is reported only once for each function it appears in > 973 | pr_info("%s", version); > | ^~~~~~~ > ../include/linux/printk.h:368:34: note: in definition of macro ‘pr_info’ > 368 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__) > | ^~~~~~~~~~~ > make[5]: *** [../scripts/Makefile.build:281: drivers/net/ethernet/8390/8390.o] Error 1 > make[4]: *** [../scripts/Makefile.build:497: drivers/net/ethernet/8390] Error 2 > make[4]: *** Waiting for unfinished jobs.... > make[3]: *** [../scripts/Makefile.build:497: drivers/net/ethernet] Error 2 > make[3]: *** Waiting for unfinished jobs.... > make[2]: *** [../scripts/Makefile.build:497: drivers/net] Error 2 > make[2]: *** Waiting for unfinished jobs.... > make[1]: *** [/netdev/net-next/Makefile:1771: drivers] Error 2 > make: *** [Makefile:185: __sub-make] Error 2 Did you apply patch 2/2 of the patchset? Because version-printing (and the need for a version string) was removed from lib8390.c in patch 2/2 to allow the replacement of said version-string with MODULE_* macros in 8390.c, and failing to do so whould result in the exact same error.
> Did you apply patch 2/2 of the patchset? > Because version-printing (and the need for a version string) was removed > from lib8390.c in patch 2/2 to allow the replacement of said > version-string with MODULE_* macros in 8390.c, and failing to do so whould result > in the exact same error. Hi Armin We require that there be no break in buildability. The kernel must always build. Otherwise git bisect becomes much more difficult to use. Andrew
diff --git a/drivers/net/ethernet/8390/8390.c b/drivers/net/ethernet/8390/8390.c index 0e0aa4016858..aabb637c1fbf 100644 --- a/drivers/net/ethernet/8390/8390.c +++ b/drivers/net/ethernet/8390/8390.c @@ -1,11 +1,26 @@ // SPDX-License-Identifier: GPL-2.0-only -/* 8390 core for usual drivers */ -static const char version[] = - "8390.c:v1.10cvs 9/23/94 Donald Becker (becker@cesdis.gsfc.nasa.gov)\n"; +#define DRV_NAME "8390" +#define DRV_DESCRIPTION "8390 core for usual drivers" +#define DRV_AUTHOR "Donald Becker (becker@cesdis.gsfc.nasa.gov)" +#define DRV_VERSION "1.10cvs" +#define DRV_RELDATE "9/23/1994" + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/export.h> + +#include <linux/netdevice.h> +#include <linux/etherdevice.h> #include "lib8390.c" +MODULE_AUTHOR(DRV_AUTHOR); +MODULE_DESCRIPTION(DRV_DESCRIPTION); +MODULE_VERSION(DRV_VERSION); +MODULE_LICENSE("GPL"); + int ei_open(struct net_device *dev) { return __ei_open(dev); @@ -64,7 +79,7 @@ const struct net_device_ops ei_netdev_ops = { .ndo_get_stats = ei_get_stats, .ndo_set_rx_mode = ei_set_multicast_list, .ndo_validate_addr = eth_validate_addr, - .ndo_set_mac_address = eth_mac_addr, + .ndo_set_mac_address = eth_mac_addr, #ifdef CONFIG_NET_POLL_CONTROLLER .ndo_poll_controller = ei_poll, #endif @@ -74,6 +89,7 @@ EXPORT_SYMBOL(ei_netdev_ops); struct net_device *__alloc_ei_netdev(int size) { struct net_device *dev = ____alloc_ei_netdev(size); + if (dev) dev->netdev_ops = &ei_netdev_ops; return dev; @@ -100,4 +116,3 @@ static void __exit ns8390_module_exit(void) module_init(ns8390_module_init); module_exit(ns8390_module_exit); #endif /* MODULE */ -MODULE_LICENSE("GPL");
Replace version string with MODULE_* macros. Include necessary libraries. Fix two minor coding-style issues. Signed-off-by: Armin Wolf <W_Armin@gmx.de> --- drivers/net/ethernet/8390/8390.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) -- 2.20.1