On Tue, Jan 22, 2019 at 06:12:31PM +0100, Andreas Färber wrote: > Am 22.01.19 um 17:56 schrieb Paul Gortmaker: > > [[PATCH v7 1/6] fieldbus_dev: add Fieldbus Device subsystem.] On 22/01/2019 > > (Tue 10:21) Sven Van Asbroeck wrote: > > > >> Fieldbus device (client) adapters allow data exchange with a PLC aka. > >> "Fieldbus Controller" over a fieldbus (Profinet, FLNet, etc.) > >> > >> They are typically used when a Linux device wants to expose itself > >> as an actuator, motor, console light, switch, etc. over the fieldbus. > >> > >> This framework is designed to provide a generic interface to Fieldbus > >> Devices from both the Linux Kernel and the userspace. > >> > >> Signed-off-by: Sven Van Asbroeck <[email protected]> > > > > [...] > > > >> diff --git a/drivers/fieldbus/Kconfig b/drivers/fieldbus/Kconfig > >> new file mode 100644 > >> index 000000000000..5c2bef950d04 > >> --- /dev/null > >> +++ b/drivers/fieldbus/Kconfig > >> @@ -0,0 +1,19 @@ > >> +menuconfig FIELDBUS_DEV > >> + bool "Fieldbus Device Support" > > > > OK, so the core support is NOT tristate, ie not modular, > > Is that intentional though, and if so, why? > > If possible, I would much rather see it changed to tristate and leave > all the metadata below. > > > so I think... > > > >> diff --git a/drivers/fieldbus/dev_core.c b/drivers/fieldbus/dev_core.c > >> new file mode 100644 > >> index 000000000000..c816df3201bb > >> --- /dev/null > >> +++ b/drivers/fieldbus/dev_core.c > >> @@ -0,0 +1,348 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* > >> + * Fieldbus Device Driver Core > >> + * > >> + */ > >> + > >> +#include <linux/fieldbus_dev.h> > >> +#include <linux/mutex.h> > >> +#include <linux/module.h> > > > > ...you don't need module.h here > > > >> +#include <linux/device.h> > >> +#include <linux/idr.h> > >> +#include <linux/fs.h> > >> +#include <linux/slab.h> > >> +#include <linux/poll.h> > >> + > > > > [...] > > > >> + > >> +static void __exit fieldbus_exit(void) > >> +{ > >> + unregister_chrdev_region(fieldbus_devt, MAX_FIELDBUSES); > >> + class_unregister(&fieldbus_class); > >> +} > >> + > >> +subsys_initcall(fieldbus_init); > >> +module_exit(fieldbus_exit); > > > > ...and the module_exit is never called, so fieldbus_exit is dead code > > and hence both should be removed. > > > >> + > >> +MODULE_AUTHOR("Sven Van Asbroeck <[email protected]>"); > >> +MODULE_AUTHOR("Jonathan Stiles <[email protected]>"); > >> +MODULE_DESCRIPTION("Fieldbus Device Driver Core"); > >> +MODULE_LICENSE("GPL v2"); > > Sven, as this code appears to be all by ArcX people, any chance you can > make it just GPL (SPDX GPL-2.0-or-later)? Only reason to have it v2-only
SPDX "GPL-2.0+" please. > would be if this code was derived from v2-only code that can't be > relicensed anymore. Are you _sure_ you want v2+? I ask as I am forced to :) And I thought when I made the comment about this code being a module, that would be the change, not this one forcing it not to be :( thanks, greg k-h
