V4L2 core I2C/SPI code cleanup
v2 cover V4L2 core I2C/SPI code cleanup
Hi Mauro, Hans:
A new spin of this cleanup series. Goal here is to remove some ifdefs and while at it remove some low hanging fruit.
Patch 1 merges v4l2-common.ko into videodev.ko, since it doesn't seem to be useful to have them splitted.
Patches 2 to 5 move the I2C and SPI helpers, so they can be conditionally built.
Patch 6 removes unneeded BUG() calls in these helpers. Hans, you suggested to replace BUG with WARN_ON, but it seemed that just returning NULL was fine.
There are a few checkpatch.pl issues triggered here, all of them belonging to the already existing code. I'm not addressing that in this series.
The entire series should not affect any functionality, but just clean-up the code a bit.
Thanks,
Eze
Ezequiel Garcia (6):
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
media: v4l2-core: Remove BUG() from i2c and spi helpers
drivers/media/v4l2-core/Kconfig | 5 + drivers/media/v4l2-core/Makefile | 5 +- drivers/media/v4l2-core/v4l2-common.c | 214 -------------------------- drivers/media/v4l2-core/v4l2-dev.c | 4 +- drivers/media/v4l2-core/v4l2-device.c | 39 +---- drivers/media/v4l2-core/v4l2-i2c.c | 168 ++++++++++++++++++++ drivers/media/v4l2-core/v4l2-spi.c | 73 +++++++++ include/media/v4l2-common.h | 150 +++++++++++++----- 8 files changed, 366 insertions(+), 292 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 v2 1/6 media: v4l2-core: Module re-organization
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 was 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 and the module split was needed. However, with the current organization, there is no issue compiling V4L2 with I2C as y/m/n.
This commit makes v4l2-common part of our V4L2 core driver (videodev.ko).
---
Changes v2:
* Fix MODULE_xxx macros, combining all the authors and amending the description.
--- drivers/media/v4l2-core/Makefile | 3 +-- drivers/media/v4l2-core/v4l2-common.c | 4 ---- drivers/media/v4l2-core/v4l2-dev.c | 4 ++-- 3 files changed, 3 insertions(+), 8 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
drivers/media/v4l2-core/v4l2-common.c +0 -4
@@ -54,10 +54,6 @@#include <linux/videodev2.h> -MODULE_AUTHOR ("Bill Dirks, Justin Schoeman, Gerd Knorr" ); -MODULE_DESCRIPTION ("misc helper functions for v4l2 device drivers" ); -MODULE_LICENSE ("GPL" ); -/* * * V 4 L 2 D R I V E R H E L P E R A P I
drivers/media/v4l2-core/v4l2-dev.c +2 -2
@@ -1092,7 +1092,7 @@ static void __exit videodev_exit(void)subsys_initcall (videodev_init); module_exit(videodev_exit) -MODULE_AUTHOR("Alan Cox, Mauro Carvalho Chehab <[email protected]>"); -MODULE_DESCRIPTION ("Device registrar for Video4Linux drivers v2" ); +MODULE_AUTHOR("Alan Cox, Mauro Carvalho Chehab <[email protected]>, Bill Dirks, Justin Schoeman, Gerd Knorr"); +MODULE_DESCRIPTION ("Video4Linux2 core driver" );MODULE_LICENSE ("GPL" );MODULE_ALIAS_CHARDEV_MAJOR (VIDEO_MAJOR );
Signature · 1 line
2.22.0
PATCH v2 2/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.
---
Changes v2:
* None.
--- 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 v2 3/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.
---
Changes v2:
* None.
--- 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 v2 4/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.
---
Changes v2:
* None.
--- 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 v2 5/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 ifdef.
---
Changes v2:
* None.
--- 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 v2 6/6 media: v4l2-core: Remove BUG() from i2c and spi helpers
In reply to Ezequiel GarciaCurrently, the i2c and spi subdev creation helpers are calling BUG() when passed a NULL v4l2_device parameter.
This makes little sense; simply returning NULL seems more sensible.
These two helpers may already return NULL on error, so callers should already be checking for this, or at least be prepared for a NULL result.
---
Changes v2:
* New patch.
--- drivers/media/v4l2-core/v4l2-i2c.c | 3 ++- drivers/media/v4l2-core/v4l2-spi.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-)
drivers/media/v4l2-core/v4l2-i2c.c +2 -1
@@ -64,7 +64,8 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,struct v4l2_subdev *sd =NULL ;struct i2c_client *client; -BUG_ON (!v4l2_dev); +if (!v4l2_dev) +return NULL ;request_module (I2C_MODULE_PREFIX "%s" , info->type);
drivers/media/v4l2-core/v4l2-spi.c +2 -2
@@ -39,8 +39,8 @@ struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,struct v4l2_subdev *sd =NULL ;struct spi_device *spi =NULL ; -BUG_ON (!v4l2_dev); - +if (!v4l2_dev) +return NULL ;if (info->modalias[0 ])request_module (info->modalias);
Signature · 1 line
2.22.0