Re: [PATCH 2/6] media: v4l2-core: Module re-organization

[Mauro Carvalho Chehab <mchehab+samsung@kernel.org>]

Em Thu, 25 Jul 2019 13:41:34 -0300
Ezequiel Garcia <ezequiel@collabora.com> escreveu:

> On Thu, 2019-07-25 at 13:31 -0300, Mauro Carvalho Chehab wrote:
> > Em Mon, 15 Jul 2019 18:06:43 -0300
> > Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> >   
> > > videodev.ko and v4l2-common.ko driver are built under
> > > the same conditions. Therefore, it doesn't make much sense
> > > to split them in two different modules.
> > > 
> > > Splitting v4l2-common to its own driver has done many
> > > years ago:
> > > 
> > >   commit a9254475bbfbed5f0596d952c6a3c9806e19dd0b
> > >   Author: Mauro Carvalho Chehab <mchehab@infradead.org>
> > >   Date:   Tue Jan 29 18:32:35 2008 -0300
> > > 
> > >       V4L/DVB (7115): Fix bug #9833: regression when compiling V4L without I2C
> > > 
> > > Back then, the subsystem organization was different.
> > > However, With the current organization, there is no issue
> > > compiling V4L2 with I2C as y/m/n.
> > > 
> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > ---
> > >  drivers/media/v4l2-core/Makefile | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> > > index 4d42418e603e..8e2f52f7800b 100644
> > > --- a/drivers/media/v4l2-core/Makefile
> > > +++ b/drivers/media/v4l2-core/Makefile
> > > @@ -7,14 +7,13 @@ tuner-objs	:=	tuner-core.o
> > >  
> > >  videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
> > >  			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
> > > -			v4l2-async.o
> > > +			v4l2-async.o v4l2-common.o
> > >  videodev-$(CONFIG_COMPAT) += v4l2-compat-ioctl32.o
> > >  videodev-$(CONFIG_TRACEPOINTS) += v4l2-trace.o
> > >  videodev-$(CONFIG_MEDIA_CONTROLLER) += v4l2-mc.o
> > >  
> > >  obj-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o
> > >  obj-$(CONFIG_VIDEO_V4L2) += videodev.o
> > > -obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
> > >  obj-$(CONFIG_VIDEO_V4L2) += v4l2-dv-timings.o
> > >  
> > >  obj-$(CONFIG_VIDEO_TUNER) += tuner.o  
> > 
> > Huh? This patch sounds incomplete... Where are you removing the
> > MODULE_foo macros) from v4l2-common.c?
> >   
> 
> If you are refering to:
> 
> MODULE_AUTHOR("Bill Dirks, Justin Schoeman, Gerd Knorr");
> MODULE_DESCRIPTION("misc helper functions for v4l2 device drivers");
> MODULE_LICENSE("GPL");
> 
> I decided to keep them for documentation purposes,
> but more importantly because they were already there
> before commit a9254475bbfbed5f0596d952c6a3c9806e19dd0b
> (when v4l2-common.c was not a module, afaics).
> 
> Do you think we should remove them?

Well, it needs to be handled somehow. Please notice, however, that
there are some differences between v4l2-common and v4l2-dev:

drivers/media/v4l2-core/v4l2-common.c:MODULE_AUTHOR("Bill Dirks, Justin Schoeman, Gerd Knorr");
drivers/media/v4l2-core/v4l2-common.c:MODULE_DESCRIPTION("misc helper functions for v4l2 device drivers");
drivers/media/v4l2-core/v4l2-common.c:MODULE_LICENSE("GPL");
drivers/media/v4l2-core/v4l2-common.c:	request_module(I2C_MODULE_PREFIX "%s", info->type);
drivers/media/v4l2-core/v4l2-dev.c:MODULE_AUTHOR("Alan Cox, Mauro Carvalho Chehab <mchehab@kernel.org>");
drivers/media/v4l2-core/v4l2-dev.c:MODULE_DESCRIPTION("Device registrar for Video4Linux drivers v2");
drivers/media/v4l2-core/v4l2-dev.c:MODULE_LICENSE("GPL");
drivers/media/v4l2-core/v4l2-dev.c:MODULE_ALIAS_CHARDEV_MAJOR(VIDEO_MAJOR);

So, you can't simply remove. You'll probably need to touch
both files, do some cleanup and keep some things under the
licensing comment at the top of the file (if not there yet).

Thanks,
Mauro