lkml.org 
[lkml]   [2019]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v7 1/6] fieldbus_dev: add Fieldbus Device subsystem.
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 <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

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

\
 
 \ /
  Last update: 2019-01-22 19:53    [W:0.629 / U:0.160 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site