| Message ID | 20230120114524.408368-7-hdegoede@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | leds: lookup-table support + int3472/media privacy LED support | expand |
Hi, On 1/20/23 13:47, Sakari Ailus wrote: > Hi Hans, > > Many thanks for working on this. You are welcome. > On Fri, Jan 20, 2023 at 12:45:19PM +0100, Hans de Goede wrote: >> Currently the videodev.ko code may be builtin while e.g. v4l2-fwnode.ko >> is build as a module. >> >> This makes it hard to add code depending on other subsystems spanning >> both videodev.ko and v4l2-fwnode.ko. Specifically this block adding code >> depending on the LED subsystem. >> >> This is made even harder because CONFIG_V4L2_FWNODE is selected, >> not depended on so it itself cannot depend on another subsystem without >> editing all the Kconfig symbols selecting it to also list the dependency >> and there are many of such symbols. >> >> Adding a "select LED_CLASS if NEW_LEDS" to CONFIG_V4L2_FWNODE leads >> to Kconfig erroring out with "error: recursive dependency detected!". >> >> To fix this dependency mess, change the V4L2_FWNODE and V4L2_ASYNC >> (which V4L2_FWNODE selects) Kconfig symbols from tristate to bools and >> link their code into videodev.ko instead of making them separate modules. >> >> This will allow using IS_REACHABLE(LED_CLASS) for the new LED integration >> code without needing to worry that it expands to 0 in some places and >> 1 in other places because some of the code being builtin vs modular. >> >> On x86_64 this leads to the following size changes for videodev.ko >> >> [hans@shalem linux]$ size drivers/media/v4l2-core/videodev.ko >> >> Before: >> text data bss dec hex filename >> 218206 14395 2448 235049 39629 drivers/media/v4l2-core/videodev.ko >> After: >> text data bss dec hex filename >> 243213 17615 2456 263284 40474 drivers/media/v4l2-core/videodev.ko >> >> So (as expected) there is some increase in size here, but it >> really is not that much. >> >> And the uncompressed no-debuginfo .ko file disk-usage actually shrinks >> by 17 KiB (comparing the slightly larger videodev.ko against the >> 3 original modules) and loading time will also be better. >> >> Acked-by: Linus Walleij <linus.walleij@linaro.org> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> Changes in v5: >> - Add a new v4l2-dev-priv.h for the async debugfs prototypes and add >> static inline wrappers there when CONFIG_V4L2_ASYNC is not enabled >> >> Changes in v4: >> - New patch in v4 of this patch-set >> --- >> drivers/media/v4l2-core/Kconfig | 4 ++-- >> drivers/media/v4l2-core/Makefile | 4 ++-- >> drivers/media/v4l2-core/v4l2-async.c | 17 ++++------------- >> drivers/media/v4l2-core/v4l2-dev-priv.h | 19 +++++++++++++++++++ >> drivers/media/v4l2-core/v4l2-dev.c | 8 ++++++++ >> drivers/media/v4l2-core/v4l2-fwnode.c | 6 ------ >> 6 files changed, 35 insertions(+), 23 deletions(-) >> create mode 100644 drivers/media/v4l2-core/v4l2-dev-priv.h >> >> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig >> index 348559bc2468..73574d946010 100644 >> --- a/drivers/media/v4l2-core/Kconfig >> +++ b/drivers/media/v4l2-core/Kconfig >> @@ -68,11 +68,11 @@ config V4L2_FLASH_LED_CLASS >> When in doubt, say N. >> >> config V4L2_FWNODE >> - tristate >> + bool >> select V4L2_ASYNC >> >> config V4L2_ASYNC >> - tristate >> + bool >> >> # Used by drivers that need Videobuf modules >> config VIDEOBUF_GEN >> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile >> index 41d91bd10cf2..8c5a1ab8d939 100644 >> --- a/drivers/media/v4l2-core/Makefile >> +++ b/drivers/media/v4l2-core/Makefile >> @@ -15,7 +15,9 @@ videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \ >> >> # Please keep it alphabetically sorted by Kconfig name >> # (e. g. LC_ALL=C sort Makefile) >> +videodev-$(CONFIG_V4L2_ASYNC) += v4l2-async.o >> videodev-$(CONFIG_COMPAT) += v4l2-compat-ioctl32.o >> +videodev-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o >> videodev-$(CONFIG_MEDIA_CONTROLLER) += v4l2-mc.o >> videodev-$(CONFIG_SPI) += v4l2-spi.o >> videodev-$(CONFIG_TRACEPOINTS) += v4l2-trace.o >> @@ -24,9 +26,7 @@ videodev-$(CONFIG_VIDEO_V4L2_I2C) += v4l2-i2c.o >> # Please keep it alphabetically sorted by Kconfig name >> # (e. g. LC_ALL=C sort Makefile) >> >> -obj-$(CONFIG_V4L2_ASYNC) += v4l2-async.o >> obj-$(CONFIG_V4L2_FLASH_LED_CLASS) += v4l2-flash-led-class.o >> -obj-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o >> obj-$(CONFIG_V4L2_H264) += v4l2-h264.o >> obj-$(CONFIG_V4L2_JPEG_HELPER) += v4l2-jpeg.o >> obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o >> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c >> index 2f1b718a9189..024d6b82b50a 100644 >> --- a/drivers/media/v4l2-core/v4l2-async.c >> +++ b/drivers/media/v4l2-core/v4l2-async.c >> @@ -11,7 +11,6 @@ >> #include <linux/i2c.h> >> #include <linux/list.h> >> #include <linux/mm.h> >> -#include <linux/module.h> >> #include <linux/mutex.h> >> #include <linux/of.h> >> #include <linux/platform_device.h> >> @@ -24,6 +23,8 @@ >> #include <media/v4l2-fwnode.h> >> #include <media/v4l2-subdev.h> >> >> +#include "v4l2-dev-priv.h" >> + >> static int v4l2_async_nf_call_bound(struct v4l2_async_notifier *n, >> struct v4l2_subdev *subdev, >> struct v4l2_async_subdev *asd) >> @@ -900,25 +901,15 @@ DEFINE_SHOW_ATTRIBUTE(pending_subdevs); >> >> static struct dentry *v4l2_async_debugfs_dir; >> >> -static int __init v4l2_async_init(void) >> +void __init v4l2_async_debugfs_init(void) >> { >> v4l2_async_debugfs_dir = debugfs_create_dir("v4l2-async", NULL); >> debugfs_create_file("pending_async_subdevices", 0444, >> v4l2_async_debugfs_dir, NULL, >> &pending_subdevs_fops); >> - >> - return 0; >> } >> >> -static void __exit v4l2_async_exit(void) >> +void __exit v4l2_async_debugfs_exit(void) >> { >> debugfs_remove_recursive(v4l2_async_debugfs_dir); >> } >> - >> -subsys_initcall(v4l2_async_init); >> -module_exit(v4l2_async_exit); >> - >> -MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>"); >> -MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>"); >> -MODULE_AUTHOR("Ezequiel Garcia <ezequiel@collabora.com>"); >> -MODULE_LICENSE("GPL"); >> diff --git a/drivers/media/v4l2-core/v4l2-dev-priv.h b/drivers/media/v4l2-core/v4l2-dev-priv.h >> new file mode 100644 >> index 000000000000..b5b1ee78be20 >> --- /dev/null >> +++ b/drivers/media/v4l2-core/v4l2-dev-priv.h > > Could we call this v4l2-async-debugfs.h? I don't necessarily expect more > material here. Renaming it is fine by my. I have renamed it to v4l2-async-debugfs.h for the upcoming v6 of this series. Regards, Hans > >> @@ -0,0 +1,19 @@ >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >> +/* >> + * Video capture interface for Linux version 2 private header. >> + * >> + * Copyright (C) 2023 Hans de Goede <hdegoede@redhat.com> >> + */ >> + >> +#ifndef _V4L2_DEV_PRIV_H_ >> +#define _V4L2_DEV_PRIV_H_ >> + >> +#if IS_ENABLED(CONFIG_V4L2_ASYNC) >> +void v4l2_async_debugfs_init(void); >> +void v4l2_async_debugfs_exit(void); >> +#else >> +static inline void v4l2_async_debugfs_init(void) {} >> +static inline void v4l2_async_debugfs_exit(void) {} >> +#endif >> + >> +#endif >> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c >> index 397d553177fa..10ba2e4196a6 100644 >> --- a/drivers/media/v4l2-core/v4l2-dev.c >> +++ b/drivers/media/v4l2-core/v4l2-dev.c >> @@ -31,6 +31,8 @@ >> #include <media/v4l2-ioctl.h> >> #include <media/v4l2-event.h> >> >> +#include "v4l2-dev-priv.h" >> + >> #define VIDEO_NUM_DEVICES 256 >> #define VIDEO_NAME "video4linux" >> >> @@ -1190,6 +1192,7 @@ static int __init videodev_init(void) >> return -EIO; >> } >> >> + v4l2_async_debugfs_init(); >> return 0; >> } >> >> @@ -1197,6 +1200,7 @@ static void __exit videodev_exit(void) >> { >> dev_t dev = MKDEV(VIDEO_MAJOR, 0); >> >> + v4l2_async_debugfs_exit(); >> class_unregister(&video_class); >> unregister_chrdev_region(dev, VIDEO_NUM_DEVICES); >> } >> @@ -1205,6 +1209,10 @@ subsys_initcall(videodev_init); >> module_exit(videodev_exit) >> >> MODULE_AUTHOR("Alan Cox, Mauro Carvalho Chehab <mchehab@kernel.org>, Bill Dirks, Justin Schoeman, Gerd Knorr"); >> +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>"); >> +MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>"); >> +MODULE_AUTHOR("Ezequiel Garcia <ezequiel@collabora.com>"); >> +MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>"); >> MODULE_DESCRIPTION("Video4Linux2 core driver"); >> MODULE_LICENSE("GPL"); >> MODULE_ALIAS_CHARDEV_MAJOR(VIDEO_MAJOR); >> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c >> index 3d9533c1b202..c8a2264262bc 100644 >> --- a/drivers/media/v4l2-core/v4l2-fwnode.c >> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c >> @@ -17,7 +17,6 @@ >> #include <linux/acpi.h> >> #include <linux/kernel.h> >> #include <linux/mm.h> >> -#include <linux/module.h> >> #include <linux/of.h> >> #include <linux/property.h> >> #include <linux/slab.h> >> @@ -1328,8 +1327,3 @@ int v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd) >> return ret; >> } >> EXPORT_SYMBOL_GPL(v4l2_async_register_subdev_sensor); >> - >> -MODULE_LICENSE("GPL"); >> -MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>"); >> -MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>"); >> -MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>"); >> -- >> 2.39.0 >> >
diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig index 348559bc2468..73574d946010 100644 --- a/drivers/media/v4l2-core/Kconfig +++ b/drivers/media/v4l2-core/Kconfig @@ -68,11 +68,11 @@ config V4L2_FLASH_LED_CLASS When in doubt, say N. config V4L2_FWNODE - tristate + bool select V4L2_ASYNC config V4L2_ASYNC - tristate + bool # Used by drivers that need Videobuf modules config VIDEOBUF_GEN diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile index 41d91bd10cf2..8c5a1ab8d939 100644 --- a/drivers/media/v4l2-core/Makefile +++ b/drivers/media/v4l2-core/Makefile @@ -15,7 +15,9 @@ videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \ # Please keep it alphabetically sorted by Kconfig name # (e. g. LC_ALL=C sort Makefile) +videodev-$(CONFIG_V4L2_ASYNC) += v4l2-async.o videodev-$(CONFIG_COMPAT) += v4l2-compat-ioctl32.o +videodev-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o videodev-$(CONFIG_MEDIA_CONTROLLER) += v4l2-mc.o videodev-$(CONFIG_SPI) += v4l2-spi.o videodev-$(CONFIG_TRACEPOINTS) += v4l2-trace.o @@ -24,9 +26,7 @@ videodev-$(CONFIG_VIDEO_V4L2_I2C) += v4l2-i2c.o # Please keep it alphabetically sorted by Kconfig name # (e. g. LC_ALL=C sort Makefile) -obj-$(CONFIG_V4L2_ASYNC) += v4l2-async.o obj-$(CONFIG_V4L2_FLASH_LED_CLASS) += v4l2-flash-led-class.o -obj-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o obj-$(CONFIG_V4L2_H264) += v4l2-h264.o obj-$(CONFIG_V4L2_JPEG_HELPER) += v4l2-jpeg.o obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index 2f1b718a9189..024d6b82b50a 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -11,7 +11,6 @@ #include <linux/i2c.h> #include <linux/list.h> #include <linux/mm.h> -#include <linux/module.h> #include <linux/mutex.h> #include <linux/of.h> #include <linux/platform_device.h> @@ -24,6 +23,8 @@ #include <media/v4l2-fwnode.h> #include <media/v4l2-subdev.h> +#include "v4l2-dev-priv.h" + static int v4l2_async_nf_call_bound(struct v4l2_async_notifier *n, struct v4l2_subdev *subdev, struct v4l2_async_subdev *asd) @@ -900,25 +901,15 @@ DEFINE_SHOW_ATTRIBUTE(pending_subdevs); static struct dentry *v4l2_async_debugfs_dir; -static int __init v4l2_async_init(void) +void __init v4l2_async_debugfs_init(void) { v4l2_async_debugfs_dir = debugfs_create_dir("v4l2-async", NULL); debugfs_create_file("pending_async_subdevices", 0444, v4l2_async_debugfs_dir, NULL, &pending_subdevs_fops); - - return 0; } -static void __exit v4l2_async_exit(void) +void __exit v4l2_async_debugfs_exit(void) { debugfs_remove_recursive(v4l2_async_debugfs_dir); } - -subsys_initcall(v4l2_async_init); -module_exit(v4l2_async_exit); - -MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>"); -MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>"); -MODULE_AUTHOR("Ezequiel Garcia <ezequiel@collabora.com>"); -MODULE_LICENSE("GPL"); diff --git a/drivers/media/v4l2-core/v4l2-dev-priv.h b/drivers/media/v4l2-core/v4l2-dev-priv.h new file mode 100644 index 000000000000..b5b1ee78be20 --- /dev/null +++ b/drivers/media/v4l2-core/v4l2-dev-priv.h @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Video capture interface for Linux version 2 private header. + * + * Copyright (C) 2023 Hans de Goede <hdegoede@redhat.com> + */ + +#ifndef _V4L2_DEV_PRIV_H_ +#define _V4L2_DEV_PRIV_H_ + +#if IS_ENABLED(CONFIG_V4L2_ASYNC) +void v4l2_async_debugfs_init(void); +void v4l2_async_debugfs_exit(void); +#else +static inline void v4l2_async_debugfs_init(void) {} +static inline void v4l2_async_debugfs_exit(void) {} +#endif + +#endif diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index 397d553177fa..10ba2e4196a6 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -31,6 +31,8 @@ #include <media/v4l2-ioctl.h> #include <media/v4l2-event.h> +#include "v4l2-dev-priv.h" + #define VIDEO_NUM_DEVICES 256 #define VIDEO_NAME "video4linux" @@ -1190,6 +1192,7 @@ static int __init videodev_init(void) return -EIO; } + v4l2_async_debugfs_init(); return 0; } @@ -1197,6 +1200,7 @@ static void __exit videodev_exit(void) { dev_t dev = MKDEV(VIDEO_MAJOR, 0); + v4l2_async_debugfs_exit(); class_unregister(&video_class); unregister_chrdev_region(dev, VIDEO_NUM_DEVICES); } @@ -1205,6 +1209,10 @@ subsys_initcall(videodev_init); module_exit(videodev_exit) MODULE_AUTHOR("Alan Cox, Mauro Carvalho Chehab <mchehab@kernel.org>, Bill Dirks, Justin Schoeman, Gerd Knorr"); +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>"); +MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>"); +MODULE_AUTHOR("Ezequiel Garcia <ezequiel@collabora.com>"); +MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>"); MODULE_DESCRIPTION("Video4Linux2 core driver"); MODULE_LICENSE("GPL"); MODULE_ALIAS_CHARDEV_MAJOR(VIDEO_MAJOR); diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c index 3d9533c1b202..c8a2264262bc 100644 --- a/drivers/media/v4l2-core/v4l2-fwnode.c +++ b/drivers/media/v4l2-core/v4l2-fwnode.c @@ -17,7 +17,6 @@ #include <linux/acpi.h> #include <linux/kernel.h> #include <linux/mm.h> -#include <linux/module.h> #include <linux/of.h> #include <linux/property.h> #include <linux/slab.h> @@ -1328,8 +1327,3 @@ int v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd) return ret; } EXPORT_SYMBOL_GPL(v4l2_async_register_subdev_sensor); - -MODULE_LICENSE("GPL"); -MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>"); -MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>"); -MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>");