Re: [PATCH 2/6] media: v4l2-core: Module re-organization
On Thu, 2019-07-25 at 14:03 -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 25 Jul 2019 13:41:34 -0300
> Ezequiel Garcia 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 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
> > > > 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
> > > > ---
> > > > 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 ");
> 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).
>
OK, I'll rebase and submit a new version trying to tackle this.
Thanks,
Ezequiel
Re: [PATCH 2/6] media: v4l2-core: Module re-organization
Em Thu, 25 Jul 2019 13:41:34 -0300
Ezequiel Garcia 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 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
> > > 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
> > > ---
> > > 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 ");
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
Re: [PATCH 2/6] media: v4l2-core: Module re-organization
On Thu, 2019-07-25 at 13:31 -0300, Mauro Carvalho Chehab wrote:
> Em Mon, 15 Jul 2019 18:06:43 -0300
> Ezequiel Garcia 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
> > 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
> > ---
> > 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?
Thanks,
Ezequiel
Re: [PATCH 2/6] media: v4l2-core: Module re-organization
Em Mon, 15 Jul 2019 18:06:43 -0300 Ezequiel Garcia 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 > 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 > --- > 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? Thanks, Mauro
