Messages in this thread | | | Subject | Re: [PATCH v7 1/6] fieldbus_dev: add Fieldbus Device subsystem. | From | Andreas Färber <> | Date | Tue, 22 Jan 2019 18:12:31 +0100 |
| |
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 <TheSven73@googlemail.com> > > [...] > >> 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 <TheSven73@googlemail.com>"); >> +MODULE_AUTHOR("Jonathan Stiles <jonathans@arcx.com>"); >> +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 would be if this code was derived from v2-only code that can't be relicensed anymore.
Thanks, Andreas
-- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg)
| |