V4L2 core I2C/SPI code cleanup
cover V4L2 core I2C/SPI code cleanup
Hi Mauro, Hans:
While reading v4l2-common.c, I came across a few ifdefs that could be cleaned-up with some minor reorganization.
Patch 1 is just cleaning the Makefile, removing ifeq/endif to make it more readable.
Patch 2 merges v4l2-common.ko into videodev.ko, which I think it's now possible. Let me know if having these two modules separated serves a purpose I'm missing.
The rest of the patches split the I2C and SPI helpers, so they can be conditionally built.
There are a few checkpatch.pl issues triggered here, all of them belonging to the already existing code. Let me know if you want me to clean that as well.
The entire series should not affect any functionality, but just clean-up the code a bit.
Thanks,
Eze
Ezequiel Garcia (6):
media: v4l2-core: Cleanup Makefile
media: v4l2-core: Module re-organization
media: v4l2-core: move spi helpers out of v4l2-common.c
media: v4l2-core: move i2c helpers out of v4l2-common.c
media: v4l2-core: introduce a helper to unregister a SPI subdev
media: v4l2-core: introduce a helper to unregister a I2C subdev
drivers/media/v4l2-core/Kconfig | 5 + drivers/media/v4l2-core/Makefile | 15 +- drivers/media/v4l2-core/v4l2-common.c | 210 -------------------------- drivers/media/v4l2-core/v4l2-device.c | 39 +---- drivers/media/v4l2-core/v4l2-i2c.c | 167 ++++++++++++++++++++ drivers/media/v4l2-core/v4l2-spi.c | 73 +++++++++ include/media/v4l2-common.h | 150 +++++++++++++----- 7 files changed, 366 insertions(+), 293 deletions(-) create mode 100644 drivers/media/v4l2-core/v4l2-i2c.c create mode 100644 drivers/media/v4l2-core/v4l2-spi.c
Signature · 1 line
2.22.0
PATCH 1/6 media: v4l2-core: Cleanup Makefile
Use the videodev-$(CONFIG_FOO) syntax to simplify the Makefile.
--- drivers/media/v4l2-core/Makefile | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
drivers/media/v4l2-core/Makefile +3 -7
@@ -8,15 +8,11 @@ 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 -ifeq ($(CONFIG_COMPAT),y) - videodev-objs += v4l2-compat-ioctl32.o -endif -obj-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o -ifeq ($(CONFIG_TRACEPOINTS),y) - videodev-objs += v4l2-trace.o -endif +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
Signature · 1 line
2.22.0
PATCH 2/6 media: v4l2-core: Module re-organization
In reply to Ezequiel Garciavideodev.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 <[email protected]>
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.
--- drivers/media/v4l2-core/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/media/v4l2-core/Makefile +1 -2
@@ -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
Signature · 1 line
2.22.0
Re: [PATCH 2/6] media: v4l2-core: Module re-organization
Em Mon, 15 Jul 2019 18:06:43 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 32 lines
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 <[email protected]>
Date: Tue Jan 29 18:32:35 2008 -0300V4L/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 <[email protected]>--- drivers/media/v4l2-core/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/media/v4l2-core/Makefile +1
-1@@ -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
drivers/media/v4l2-core/Makefile tuner-objs := tuner-core.o · line 19
Quoted 10 lines
drivers/media/v4l2-core/Makefile +0
-1videodev-$(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
Re: [PATCH 2/6] media: v4l2-core: Module re-organization
Quoted 44 lines · Mauro Carvalho Chehab
Em Mon, 15 Jul 2019 18:06:43 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 41 lines
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 <[email protected]>
Date: Tue Jan 29 18:32:35 2008 -0300V4L/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 <[email protected]>--- drivers/media/v4l2-core/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/media/v4l2-core/Makefile +1
-2@@ -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
Quoted 5 lines
drivers/media/v4l2-core/Makefile tuner-objs := tuner-core.o · line 19
Quoted 1 line
drivers/media/v4l2-core/Makefile +0
-0obj-$(CONFIG_VIDEO_TUNER) += tuner.oHuh? This patch sounds incomplete... Where are you removing the
MODULE_foomacros) fromv4l2-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 Thu, 25 Jul 2019 13:41:34 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 57 lines · Mauro Carvalho Chehab
Quoted 44 lines
Em Mon, 15 Jul 2019 18:06:43 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 41 lines
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 <[email protected]>
Date: Tue Jan 29 18:32:35 2008 -0300V4L/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 <[email protected]>--- drivers/media/v4l2-core/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/media/v4l2-core/Makefile +1
-2@@ -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.oQuoted 5 lines
drivers/media/v4l2-core/Makefile tuner-objs := tuner-core.o · line 19
Quoted 1 line
drivers/media/v4l2-core/Makefile +0
-0obj-$(CONFIG_VIDEO_TUNER) += tuner.oHuh? This patch sounds incomplete... Where are you removing the
MODULE_foomacros) fromv4l2-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");
Quoted 6 lines
I decided to keep them for documentation purposes, but more importantly because they were already there before commit
a9254475bbfbed5f0596d952c6a3c9806e19dd0b(whenv4l2-common.cwas 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 <[email protected]>"); 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
Quoted 79 lines · Mauro Carvalho Chehab
Em Thu, 25 Jul 2019 13:41:34 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 57 lines · Mauro Carvalho Chehab
Quoted 44 lines
Em Mon, 15 Jul 2019 18:06:43 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 41 lines
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 <[email protected]>
Date: Tue Jan 29 18:32:35 2008 -0300V4L/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 <[email protected]>--- drivers/media/v4l2-core/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/media/v4l2-core/Makefile +1
-2@@ -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.oQuoted 5 lines
drivers/media/v4l2-core/Makefile tuner-objs := tuner-core.o · line 19
Quoted 1 line
drivers/media/v4l2-core/Makefile +0
-0obj-$(CONFIG_VIDEO_TUNER) += tuner.oHuh? This patch sounds incomplete... Where are you removing the
MODULE_foomacros) fromv4l2-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");Quoted 6 lines
I decided to keep them for documentation purposes, but more importantly because they were already there before commit
a9254475bbfbed5f0596d952c6a3c9806e19dd0b(whenv4l2-common.cwas 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 <[email protected]>"); 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);
Quoted 4 lines
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
PATCH 3/6 media: v4l2-core: move spi helpers out of v4l2-common.c
In reply to Ezequiel GarciaSeparate the spi helpers to v4l2-spi.c, in order to get rid of the ifdefery. No functional changes intended, this is just a cosmetic change to organize the code better.
--- drivers/media/v4l2-core/Makefile | 1 + drivers/media/v4l2-core/v4l2-common.c | 65 --------------------------- drivers/media/v4l2-core/v4l2-spi.c | 65 +++++++++++++++++++++++++++ include/media/v4l2-common.h | 18 +++++++- 4 files changed, 82 insertions(+), 67 deletions(-) create mode 100644 drivers/media/v4l2-core/v4l2-spi.c
drivers/media/v4l2-core/Makefile +1 -0
@@ -11,6 +11,7 @@ videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \ videodev-$(CONFIG_COMPAT) += v4l2-compat-ioctl32.o videodev-$(CONFIG_TRACEPOINTS) += v4l2-trace.o videodev-$(CONFIG_MEDIA_CONTROLLER) += v4l2-mc.o +videodev-$(CONFIG_SPI) += v4l2-spi.o obj-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o obj-$(CONFIG_VIDEO_V4L2) += videodev.o
drivers/media/v4l2-core/v4l2-common.c +0 -65
77 lines not on this page · raw message
drivers/media/v4l2-core/v4l2-spi.c new file +65 -0
65 lines not on this page · raw message
include/media/v4l2-common.h +16 -2
@@ -216,11 +216,10 @@ const unsigned short *v4l2_i2c_tuner_addrs(enum v4l2_i2c_tuner_type type);/* ------------------------------------------------------------------------- */ /* SPI Helper functions */ -#if defined(CONFIG_SPI )#include <linux/spi/spi.h> -struct spi_device ; +#if defined(CONFIG_SPI )/** * v4l2_spi_new_subdev - Load an spi module and return an initialized
@@ -246,6 +245,21 @@ struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,*/ void v4l2_spi_subdev_init (struct v4l2_subdev *sd,struct spi_device *spi,const struct v4l2_subdev_ops *ops); + +#else + +static inline struct v4l2_subdev * +v4l2_spi_new_subdev (struct v4l2_device *v4l2_dev, +struct spi_master *master,struct spi_board_info *info) +{ +return NULL ; +} + +static inline void +v4l2_spi_subdev_init (struct v4l2_subdev *sd,struct spi_device *spi, +const struct v4l2_subdev_ops *ops) +{} +#endif /* ------------------------------------------------------------------------- */
Signature · 1 line
2.22.0
PATCH 4/6 media: v4l2-core: move i2c helpers out of v4l2-common.c
In reply to Ezequiel GarciaSeparate the i2c helpers to v4l2-i2c.c, in order to get rid of the ifdefery. No functional changes intended, this is just a cosmetic change to organize the code better.
Given I2C is a tristate symbol, a hidden boolean symbol is introduced, to make the conditional build easier.
--- drivers/media/v4l2-core/Kconfig | 5 + drivers/media/v4l2-core/Makefile | 1 + drivers/media/v4l2-core/v4l2-common.c | 145 ------------------------- drivers/media/v4l2-core/v4l2-i2c.c | 147 ++++++++++++++++++++++++++ include/media/v4l2-common.h | 113 +++++++++++++------- 5 files changed, 229 insertions(+), 182 deletions(-) create mode 100644 drivers/media/v4l2-core/v4l2-i2c.c
drivers/media/v4l2-core/Kconfig +5 -0
@@ -11,6 +11,11 @@ config VIDEO_V4L2 select VIDEOBUF2_V4L2 if VIDEOBUF2_CORE default (I2C || I2C=n) && VIDEO_DEV +config VIDEO_V4L2_I2C + bool + depends on I2C && VIDEO_V4L2 + default y + config VIDEO_ADV_DEBUG bool "Enable advanced debug functionality on V4L2 drivers" help
drivers/media/v4l2-core/Makefile +1 -0
@@ -12,6 +12,7 @@ videodev-$(CONFIG_COMPAT) += v4l2-compat-ioctl32.o videodev-$(CONFIG_TRACEPOINTS) += v4l2-trace.o videodev-$(CONFIG_MEDIA_CONTROLLER) += v4l2-mc.o videodev-$(CONFIG_SPI) += v4l2-spi.o +videodev-$(CONFIG_VIDEO_V4L2_I2C) += v4l2-i2c.o obj-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o obj-$(CONFIG_VIDEO_V4L2) += videodev.o
drivers/media/v4l2-core/v4l2-common.c +0 -145
157 lines not on this page · raw message
drivers/media/v4l2-core/v4l2-i2c.c new file +147 -0
147 lines not on this page · raw message
include/media/v4l2-common.h +76 -37
141 lines not on this page · raw message
Signature · 1 line
2.22.0
PATCH 5/6 media: v4l2-core: introduce a helper to unregister a SPI subdev
In reply to Ezequiel GarciaIntroduce a new video4linux2 SPI helper, to unregister a subdev. This allows to get rid of some more ifdefs.
--- drivers/media/v4l2-core/v4l2-device.c | 14 ++------------ drivers/media/v4l2-core/v4l2-spi.c | 8 ++++++++ include/media/v4l2-common.h | 9 +++++++++ 3 files changed, 19 insertions(+), 12 deletions(-)
drivers/media/v4l2-core/v4l2-device.c +2 -12
@@ -11,9 +11,6 @@#include <linux/module.h> #include <linux/i2c.h> #include <linux/slab.h> -#if defined(CONFIG_SPI ) -#include <linux/spi/spi.h> -#endif #include <linux/videodev2.h> #include <media/v4l2-device.h> #include <media/v4l2-ctrls.h>
@@ -124,15 +121,8 @@ void v4l2_device_unregister(struct v4l2_device *v4l2_dev)continue ; }#endif -#if defined(CONFIG_SPI ) -if (sd->flags &V4L2_SUBDEV_FL_IS_SPI ) { -struct spi_device *spi =v4l2_get_subdevdata (sd); - -if (spi && !spi->dev.of_node && !spi->dev.fwnode) -spi_unregister_device (spi); -continue ; - } -#endif + else if (sd->flags & V4L2_SUBDEV_FL_IS_SPI) + v4l2_spi_subdev_unregister(sd); }/* Mark as unregistered, thus preventing duplicate unregistrations */ v4l2_dev->name[0 ] ='\0' ;
drivers/media/v4l2-core/v4l2-spi.c +8 -0
@@ -8,6 +8,14 @@#include <media/v4l2-common.h> #include <media/v4l2-device.h> +void v4l2_spi_subdev_unregister (struct v4l2_subdev *sd) +{ +struct spi_device *spi =v4l2_get_subdevdata (sd); + +if (spi && !spi->dev.of_node && !spi->dev.fwnode) +spi_unregister_device (spi); +} +void v4l2_spi_subdev_init (struct v4l2_subdev *sd,struct spi_device *spi,const struct v4l2_subdev_ops *ops) {
include/media/v4l2-common.h +9 -0
@@ -285,6 +285,13 @@ struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,void v4l2_spi_subdev_init (struct v4l2_subdev *sd,struct spi_device *spi,const struct v4l2_subdev_ops *ops); +/** +* v4l2_spi_subdev_unregister - Unregister a v4l2_subdev +* +* @sd: pointer to &struct v4l2_subdev +*/ +void v4l2_spi_subdev_unregister (struct v4l2_subdev *sd); +#else static inline struct v4l2_subdev *
@@ -299,6 +306,8 @@ v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi, const struct v4l2_subdev_ops *ops) {} +static inline void v4l2_spi_subdev_unregister (struct v4l2_subdev *sd) +{}#endif /* ------------------------------------------------------------------------- */
Signature · 1 line
2.22.0
PATCH 5/6 media: v4l2-core: introduce an unregister spi subdev helper
In reply to Ezequiel GarciaIntroduce a new video4linux2 SPI helper, to unregister a subdev. This allows to get rid of some more ifdefs.
--- drivers/media/v4l2-core/v4l2-device.c | 14 ++------------ drivers/media/v4l2-core/v4l2-spi.c | 8 ++++++++ include/media/v4l2-common.h | 5 +++++ 3 files changed, 15 insertions(+), 12 deletions(-)
drivers/media/v4l2-core/v4l2-device.c +2 -12
@@ -11,9 +11,6 @@#include <linux/module.h> #include <linux/i2c.h> #include <linux/slab.h> -#if defined(CONFIG_SPI ) -#include <linux/spi/spi.h> -#endif #include <linux/videodev2.h> #include <media/v4l2-device.h> #include <media/v4l2-ctrls.h>
@@ -124,15 +121,8 @@ void v4l2_device_unregister(struct v4l2_device *v4l2_dev)continue ; }#endif -#if defined(CONFIG_SPI ) -if (sd->flags &V4L2_SUBDEV_FL_IS_SPI ) { -struct spi_device *spi =v4l2_get_subdevdata (sd); - -if (spi && !spi->dev.of_node && !spi->dev.fwnode) -spi_unregister_device (spi); -continue ; - } -#endif + else if (sd->flags & V4L2_SUBDEV_FL_IS_SPI) + v4l2_spi_subdev_unregister(sd); }/* Mark as unregistered, thus preventing duplicate unregistrations */ v4l2_dev->name[0 ] ='\0' ;
drivers/media/v4l2-core/v4l2-spi.c +8 -0
@@ -8,6 +8,14 @@#include <media/v4l2-common.h> #include <media/v4l2-device.h> +void v4l2_spi_subdev_unregister (struct v4l2_subdev *sd) +{ +struct spi_device *spi =v4l2_get_subdevdata (sd); + +if (spi && !spi->dev.of_node && !spi->dev.fwnode) +spi_unregister_device (spi); +} +void v4l2_spi_subdev_init (struct v4l2_subdev *sd,struct spi_device *spi,const struct v4l2_subdev_ops *ops) {
include/media/v4l2-common.h +5 -0
@@ -249,6 +249,11 @@ struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,*/ void v4l2_spi_subdev_init (struct v4l2_subdev *sd,struct spi_device *spi,const struct v4l2_subdev_ops *ops); + +void v4l2_spi_subdev_unregister (struct v4l2_subdev *sd); +#else +static inline void v4l2_spi_subdev_unregister (struct v4l2_subdev *sd) +{}#endif /* ------------------------------------------------------------------------- */
Signature · 1 line
2.22.0
PATCH 6/6 media: v4l2-core: introduce a helper to unregister a I2C subdev
In reply to Ezequiel GarciaIntroduce a new video4linux2 I2C helper, to unregister a subdev. This allows to get rid of yet another ifdefs.
--- drivers/media/v4l2-core/v4l2-device.c | 25 ++----------------------- drivers/media/v4l2-core/v4l2-i2c.c | 20 ++++++++++++++++++++ include/media/v4l2-common.h | 10 ++++++++++ 3 files changed, 32 insertions(+), 23 deletions(-)
drivers/media/v4l2-core/v4l2-device.c +2 -23
@@ -9,7 +9,6 @@#include <linux/types.h> #include <linux/ioctl.h> #include <linux/module.h> -#include <linux/i2c.h> #include <linux/slab.h> #include <linux/videodev2.h> #include <media/v4l2-device.h>
@@ -99,28 +98,8 @@ void v4l2_device_unregister(struct v4l2_device *v4l2_dev)/* Unregister subdevs */ list_for_each_entry_safe (sd, next, &v4l2_dev->subdevs, list) {v4l2_device_unregister_subdev (sd); -#if IS_ENABLED (CONFIG_I2C ) -if (sd->flags &V4L2_SUBDEV_FL_IS_I2C ) { -struct i2c_client *client =v4l2_get_subdevdata (sd); - -/* -* We need to unregister the i2c client -* explicitly. We cannot rely on -* i2c_del_adapter to always unregister -* clients for us, since if the i2c bus is a -* platform bus, then it is never deleted. -* -* Device tree or ACPI based devices must not -* be unregistered as they have not been -* registered by us, and would not be -* re-created by just probing the V4L2 driver. -*/ -if (client && - !client->dev.of_node && !client->dev.fwnode) -i2c_unregister_device (client); -continue ; - } -#endif +if (sd->flags &V4L2_SUBDEV_FL_IS_I2C ) +v4l2_i2c_subdev_unregister (sd);else if (sd->flags &V4L2_SUBDEV_FL_IS_SPI )v4l2_spi_subdev_unregister (sd); }
drivers/media/v4l2-core/v4l2-i2c.c +20 -0
@@ -8,6 +8,26 @@#include <media/v4l2-common.h> #include <media/v4l2-device.h> +void v4l2_i2c_subdev_unregister (struct v4l2_subdev *sd) +{ +struct i2c_client *client =v4l2_get_subdevdata (sd); + +/* +* We need to unregister the i2c client +* explicitly. We cannot rely on +* i2c_del_adapter to always unregister +* clients for us, since if the i2c bus is a +* platform bus, then it is never deleted. +* +* Device tree or ACPI based devices must not +* be unregistered as they have not been +* registered by us, and would not be +* re-created by just probing the V4L2 driver. +*/ +if (client && !client->dev.of_node && !client->dev.fwnode) +i2c_unregister_device (client); +} +void v4l2_i2c_subdev_set_name (struct v4l2_subdev *sd,struct i2c_client *client,const char *devname,const char *postfix) {
include/media/v4l2-common.h +10 -0
@@ -211,6 +211,13 @@ unsigned short v4l2_i2c_subdev_addr(struct v4l2_subdev *sd);*/ const unsigned short *v4l2_i2c_tuner_addrs (enum v4l2_i2c_tuner_type type); +/** +* v4l2_i2c_subdev_unregister - Unregister a v4l2_subdev +* +* @sd: pointer to &struct v4l2_subdev +*/ +void v4l2_i2c_subdev_unregister (struct v4l2_subdev *sd); +#else static inline struct v4l2_subdev *
@@ -250,6 +257,9 @@ v4l2_i2c_tuner_addrs(enum v4l2_i2c_tuner_type type) return NULL; } +static inline void v4l2_i2c_subdev_unregister (struct v4l2_subdev *sd) +{} +#endif /* ------------------------------------------------------------------------- */
Signature · 1 line
2.22.0
PATCH 6/6 media: v4l2-core: introduce unregister subdev i2c helper
In reply to Ezequiel GarciaIntroduce a new video4linux2 I2C helper, to unregister a subdev. This allows to get rid of yet another ifdefs.
--- drivers/media/v4l2-core/v4l2-device.c | 25 ++----------------------- drivers/media/v4l2-core/v4l2-i2c.c | 20 ++++++++++++++++++++ include/media/v4l2-common.h | 4 ++++ 3 files changed, 26 insertions(+), 23 deletions(-)
drivers/media/v4l2-core/v4l2-device.c +2 -23
@@ -9,7 +9,6 @@#include <linux/types.h> #include <linux/ioctl.h> #include <linux/module.h> -#include <linux/i2c.h> #include <linux/slab.h> #include <linux/videodev2.h> #include <media/v4l2-device.h>
@@ -99,28 +98,8 @@ void v4l2_device_unregister(struct v4l2_device *v4l2_dev)/* Unregister subdevs */ list_for_each_entry_safe (sd, next, &v4l2_dev->subdevs, list) {v4l2_device_unregister_subdev (sd); -#if IS_ENABLED (CONFIG_I2C ) -if (sd->flags &V4L2_SUBDEV_FL_IS_I2C ) { -struct i2c_client *client =v4l2_get_subdevdata (sd); - -/* -* We need to unregister the i2c client -* explicitly. We cannot rely on -* i2c_del_adapter to always unregister -* clients for us, since if the i2c bus is a -* platform bus, then it is never deleted. -* -* Device tree or ACPI based devices must not -* be unregistered as they have not been -* registered by us, and would not be -* re-created by just probing the V4L2 driver. -*/ -if (client && - !client->dev.of_node && !client->dev.fwnode) -i2c_unregister_device (client); -continue ; - } -#endif +if (sd->flags &V4L2_SUBDEV_FL_IS_I2C ) +v4l2_i2c_subdev_unregister (sd);else if (sd->flags &V4L2_SUBDEV_FL_IS_SPI )v4l2_spi_subdev_unregister (sd); }
drivers/media/v4l2-core/v4l2-i2c.c +20 -0
@@ -8,6 +8,26 @@#include <media/v4l2-common.h> #include <media/v4l2-device.h> +void v4l2_i2c_subdev_unregister (struct v4l2_subdev *sd) +{ +struct i2c_client *client =v4l2_get_subdevdata (sd); + +/* +* We need to unregister the i2c client +* explicitly. We cannot rely on +* i2c_del_adapter to always unregister +* clients for us, since if the i2c bus is a +* platform bus, then it is never deleted. +* +* Device tree or ACPI based devices must not +* be unregistered as they have not been +* registered by us, and would not be +* re-created by just probing the V4L2 driver. +*/ +if (client && !client->dev.of_node && !client->dev.fwnode) +i2c_unregister_device (client); +} +void v4l2_i2c_subdev_set_name (struct v4l2_subdev *sd,struct i2c_client *client,const char *devname,const char *postfix) {
include/media/v4l2-common.h +4 -0
@@ -214,6 +214,10 @@ enum v4l2_i2c_tuner_type {*/ const unsigned short *v4l2_i2c_tuner_addrs (enum v4l2_i2c_tuner_type type); +void v4l2_i2c_subdev_unregister (struct v4l2_subdev *sd); +#else +static inline void v4l2_i2c_subdev_unregister (struct v4l2_subdev *sd) +{}#endif /* ------------------------------------------------------------------------- */
Signature · 1 line
2.22.0
Quoted 16 lines · Ezequiel Garcia
Hi Mauro, Hans:
While reading
v4l2-common.c, I came across a few ifdefs that could be cleaned-up with some minor reorganization.Patch 1 is just cleaning the
Makefile, removing ifeq/endif to make it more readable.Patch 2 merges v4l2-common.ko into videodev.ko, which I think it's now possible. Let me know if having these two modules separated serves a purpose I'm missing.
The rest of the patches split the I2C and SPI helpers, so they can be conditionally built.
Quoted 6 lines
There are a few checkpatch.pl issues triggered here, all of them belonging to the already existing code. Let me know if you want me to clean that as well.
The entire series should not affect any functionality, but just clean-up the code a bit.
This series looks good, but since you are looking in this area anyway, can you post a follow-up patch that replaces the BUG_ON in v4l2-i2c.c and -spi.c with a WARN_ON?
Thanks!
Hans
Quoted 23 lines
Thanks,
EzeEzequiel Garcia (6):
media: v4l2-core: CleanupMakefile
media: v4l2-core: Module re-organization
media: v4l2-core: move spi helpers out ofv4l2-common.c
media: v4l2-core: move i2c helpers out ofv4l2-common.c
media: v4l2-core: introduce a helper to unregister a SPI subdev
media: v4l2-core: introduce a helper to unregister a I2C subdevdrivers/media/v4l2-core/Kconfig | 5 + drivers/media/v4l2-core/Makefile | 15 +- drivers/media/v4l2-core/v4l2-common.c | 210 -------------------------- drivers/media/v4l2-core/v4l2-device.c | 39 +---- drivers/media/v4l2-core/v4l2-i2c.c | 167 ++++++++++++++++++++ drivers/media/v4l2-core/v4l2-spi.c | 73 +++++++++ include/media/v4l2-common.h | 150 +++++++++++++----- 7 files changed, 366 insertions(+), 293 deletions(-) create mode 100644 drivers/media/v4l2-core/v4l2-i2c.c create mode 100644 drivers/media/v4l2-core/v4l2-spi.c