Coding style cleanups after the fwnode patchset
Subsystems (2)
v2 cover Coding style cleanups after the fwnode patchset
The fwnode patchset added a several new warnings, as identified by checkpatch.pl --strict.
Those are at the core stuff, and makes harder to review patches there.
In order to fully address checkpatch.pl, we still need to:
- Add SPDX headers;
- Rename functions.
Let's do those on some future change.
-
v2:
- fixed a weird line break at patch 1/3;
- kept the order of arguments inside a function at patch 3/3.
Mauro Carvalho Chehab (3):
media: v4l2-core: cleanup coding style at V4L2 async/fwnode
media: v4l2-fwnode: cleanup functions that parse endpoints
media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props()
call
drivers/media/v4l2-core/v4l2-async.c | 45 +++--- drivers/media/v4l2-core/v4l2-fwnode.c | 188 ++++++++++++++------------ include/media/v4l2-async.h | 12 +- include/media/v4l2-fwnode.h | 45 +++--- include/media/v4l2-mediabus.h | 32 +++-- 5 files changed, 176 insertions(+), 146 deletions(-)
Signature · 1 line
2.17.1
PATCH v2 1/3 media: v4l2-core: cleanup coding style at V4L2 async/fwnode
There are several coding style issues at those definitions, and the previous patchset added even more.
Address the trivial ones by first calling:
./scripts/checkpatch.pl --strict --fix-inline include/media/v4l2-async.h include/media/v4l2-fwnode.h include/media/v4l2-mediabus.h drivers/media/v4l2-core/v4l2-async.c drivers/media/v4l2-core/v4l2-fwnode.c
and then manually adjusting the style where needed.
--- drivers/media/v4l2-core/v4l2-async.c | 45 ++++--- drivers/media/v4l2-core/v4l2-fwnode.c | 185 +++++++++++++++----------- include/media/v4l2-async.h | 12 +- include/media/v4l2-fwnode.h | 46 ++++--- include/media/v4l2-mediabus.h | 32 +++-- 5 files changed, 179 insertions(+), 141 deletions(-)
drivers/media/v4l2-core/v4l2-async.c +25 -20
@@ -57,6 +57,7 @@ static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd) {#if IS_ENABLED (CONFIG_I2C )struct i2c_client *client =i2c_verify_client (sd->dev); +return client && asd->match.i2c.adapter_id == client->adapter->nr && asd->match.i2c.address == client->addr;
@@ -89,10 +90,11 @@ static LIST_HEAD(subdev_list);static LIST_HEAD (notifier_list);static DEFINE_MUTEX (list_lock); -static struct v4l2_async_subdev *v4l2_async_find_match ( -struct v4l2_async_notifier *notifier,struct v4l2_subdev *sd) +static struct v4l2_async_subdev * +v4l2_async_find_match (struct v4l2_async_notifier *notifier, +struct v4l2_subdev *sd) { -bool (*match)(struct v4l2_subdev *,struct v4l2_async_subdev *); +bool (*match)(struct v4l2_subdev *sd,struct v4l2_async_subdev *asd);struct v4l2_async_subdev *asd;list_for_each_entry (asd, ¬ifier->waiting, list) {
@@ -150,8 +152,8 @@ static bool asd_equal(struct v4l2_async_subdev *asd_x, }/* Find the sub-device notifier registered by a sub-device driver. */ -static struct v4l2_async_notifier *v4l2_async_find_subdev_notifier ( -struct v4l2_subdev *sd) +static struct v4l2_async_notifier * +v4l2_async_find_subdev_notifier (struct v4l2_subdev *sd) {struct v4l2_async_notifier *n;
@@ -163,8 +165,8 @@ static struct v4l2_async_notifier *v4l2_async_find_subdev_notifier( }/* Get v4l2_device related to the notifier if one can be found. */ -static struct v4l2_device *v4l2_async_notifier_find_v4l2_dev ( -struct v4l2_async_notifier *notifier) +static struct v4l2_device * +v4l2_async_notifier_find_v4l2_dev (struct v4l2_async_notifier *notifier) {while (notifier->parent) notifier = notifier->parent;
@@ -175,8 +177,8 @@ static struct v4l2_device *v4l2_async_notifier_find_v4l2_dev(/* * Return true if all child sub-device notifiers are complete, false otherwise. */ -static bool v4l2_async_notifier_can_complete ( -struct v4l2_async_notifier *notifier) +static bool +v4l2_async_notifier_can_complete (struct v4l2_async_notifier *notifier) {struct v4l2_subdev *sd;
@@ -199,8 +201,8 @@ static bool v4l2_async_notifier_can_complete(* Complete the master notifier if possible. This is done when all async * sub-devices have been bound; v4l2_device is also available then. */ -static int v4l2_async_notifier_try_complete ( -struct v4l2_async_notifier *notifier) +static int +v4l2_async_notifier_try_complete (struct v4l2_async_notifier *notifier) {/* Quick check whether there are still more sub-devices here. */ if (!list_empty (¬ifier->waiting))
@@ -221,8 +223,8 @@ static int v4l2_async_notifier_try_complete(return v4l2_async_notifier_call_complete (notifier); } -static int v4l2_async_notifier_try_all_subdevs ( -struct v4l2_async_notifier *notifier); +static int +v4l2_async_notifier_try_all_subdevs (struct v4l2_async_notifier *notifier); static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier, struct v4l2_device *v4l2_dev,
@@ -268,8 +270,8 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier, }/* Test all async sub-devices in a notifier for a match. */ -static int v4l2_async_notifier_try_all_subdevs ( -struct v4l2_async_notifier *notifier) +static int +v4l2_async_notifier_try_all_subdevs (struct v4l2_async_notifier *notifier) {struct v4l2_device *v4l2_dev =v4l2_async_notifier_find_v4l2_dev (notifier);
@@ -306,14 +308,17 @@ static int v4l2_async_notifier_try_all_subdevs(static void v4l2_async_cleanup (struct v4l2_subdev *sd) {v4l2_device_unregister_subdev (sd); -/* Subdevice driver will reprobe and put the subdev back onto the list */ +/* +* Subdevice driver will reprobe and put the subdev back +* onto the list +*/ list_del_init (&sd->async_list); sd->asd =NULL ; }/* Unbind all sub-devices in the notifier tree. */ -static void v4l2_async_notifier_unbind_all_subdevs ( -struct v4l2_async_notifier *notifier) +static void +v4l2_async_notifier_unbind_all_subdevs (struct v4l2_async_notifier *notifier) {struct v4l2_subdev *sd, *tmp;
@@ -508,8 +513,8 @@ int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd, }EXPORT_SYMBOL (v4l2_async_subdev_notifier_register); -static void __v4l2_async_notifier_unregister ( -struct v4l2_async_notifier *notifier) +static void +__v4l2_async_notifier_unregister (struct v4l2_async_notifier *notifier) {if (!notifier || (!notifier->v4l2_dev && !notifier->sd))return ;
drivers/media/v4l2-core/v4l2-fwnode.c +106 -79
320 lines not on this page · raw message
include/media/v4l2-async.h +6 -6
@@ -89,8 +89,8 @@ struct v4l2_async_subdev {unsigned short address; } i2c;struct { -bool (*match)(struct device *, -struct v4l2_async_subdev *); +bool (*match)(struct device *dev, +struct v4l2_async_subdev *sd);void *priv; } custom; } match;
@@ -222,7 +222,6 @@ v4l2_async_notifier_add_devname_subdev(struct v4l2_async_notifier *notifier,const char *device_name, unsigned int asd_struct_size); -/** * v4l2_async_notifier_register - registers a subdevice asynchronous notifier *
@@ -243,7 +242,8 @@ int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd, struct v4l2_async_notifier *notifier);/** -* v4l2_async_notifier_unregister - unregisters a subdevice asynchronous notifier +* v4l2_async_notifier_unregister - unregisters a subdevice +* asynchronous notifier * * @notifier: pointer to &struct v4l2_async_notifier */
@@ -294,8 +294,8 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd);* An error is returned if the module is no longer loaded on any attempts * to register it. */ -int __must_check v4l2_async_register_subdev_sensor_common ( -struct v4l2_subdev *sd); +int __must_check +v4l2_async_register_subdev_sensor_common (struct v4l2_subdev *sd);/** * v4l2_async_unregister_subdev - unregisters a sub-device to the asynchronous
include/media/v4l2-fwnode.h +24 -22
93 lines not on this page · raw message
include/media/v4l2-mediabus.h +18 -14
@@ -14,7 +14,6 @@#include <linux/v4l2-mediabus.h> #include <linux/bitops.h> -/* Parallel flags */ /* * Can the client run in master or in slave mode. By "Master mode" an operation
@@ -63,10 +62,14 @@#define V4L2_MBUS_CSI2_CONTINUOUS_CLOCK BIT(8)#define V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK BIT(9) -#define V4L2_MBUS_CSI2_LANES (V4L2_MBUS_CSI2_1_LANE | V4L2_MBUS_CSI2_2_LANE | \ - V4L2_MBUS_CSI2_3_LANE | V4L2_MBUS_CSI2_4_LANE) -#define V4L2_MBUS_CSI2_CHANNELS (V4L2_MBUS_CSI2_CHANNEL_0 | V4L2_MBUS_CSI2_CHANNEL_1 | \ - V4L2_MBUS_CSI2_CHANNEL_2 | V4L2_MBUS_CSI2_CHANNEL_3) +#define V4L2_MBUS_CSI2_LANES (V4L2_MBUS_CSI2_1_LANE | \ + V4L2_MBUS_CSI2_2_LANE | \ + V4L2_MBUS_CSI2_3_LANE | \ + V4L2_MBUS_CSI2_4_LANE) +#define V4L2_MBUS_CSI2_CHANNELS (V4L2_MBUS_CSI2_CHANNEL_0 | \ + V4L2_MBUS_CSI2_CHANNEL_1 | \ + V4L2_MBUS_CSI2_CHANNEL_2 | \ + V4L2_MBUS_CSI2_CHANNEL_3)/** * enum v4l2_mbus_type - media bus type
@@ -106,8 +109,9 @@ struct v4l2_mbus_config {* @pix_fmt: pointer to &struct v4l2_pix_format to be filled * @mbus_fmt: pointer to &struct v4l2_mbus_framefmt to be used as model */ -static inline void v4l2_fill_pix_format (struct v4l2_pix_format *pix_fmt, -const struct v4l2_mbus_framefmt *mbus_fmt) +static inline void +v4l2_fill_pix_format (struct v4l2_pix_format *pix_fmt, +const struct v4l2_mbus_framefmt *mbus_fmt) { pix_fmt->width = mbus_fmt->width; pix_fmt->height = mbus_fmt->height;
@@ -128,7 +132,7 @@ static inline void v4l2_fill_pix_format(struct v4l2_pix_format *pix_fmt,* @code: data format code (from &enum v4l2_mbus_pixelcode) */ static inline void v4l2_fill_mbus_format (struct v4l2_mbus_framefmt *mbus_fmt, -const struct v4l2_pix_format *pix_fmt, +const struct v4l2_pix_format *pix_fmt,u32 code) { mbus_fmt->width = pix_fmt->width;
@@ -148,9 +152,9 @@ static inline void v4l2_fill_mbus_format(struct v4l2_mbus_framefmt *mbus_fmt,* @pix_mp_fmt: pointer to &struct v4l2_pix_format_mplane to be filled * @mbus_fmt: pointer to &struct v4l2_mbus_framefmt to be used as model */ -static inline void v4l2_fill_pix_format_mplane ( -struct v4l2_pix_format_mplane *pix_mp_fmt, -const struct v4l2_mbus_framefmt *mbus_fmt) +static inline void +v4l2_fill_pix_format_mplane (struct v4l2_pix_format_mplane *pix_mp_fmt, +const struct v4l2_mbus_framefmt *mbus_fmt) { pix_mp_fmt->width = mbus_fmt->width; pix_mp_fmt->height = mbus_fmt->height;
@@ -168,9 +172,9 @@ static inline void v4l2_fill_pix_format_mplane(* @mbus_fmt: pointer to &struct v4l2_mbus_framefmt to be filled * @pix_mp_fmt: pointer to &struct v4l2_pix_format_mplane to be used as model */ -static inline void v4l2_fill_mbus_format_mplane ( -struct v4l2_mbus_framefmt *mbus_fmt, -const struct v4l2_pix_format_mplane *pix_mp_fmt) +static inline void +v4l2_fill_mbus_format_mplane (struct v4l2_mbus_framefmt *mbus_fmt, +const struct v4l2_pix_format_mplane *pix_mp_fmt) { mbus_fmt->width = pix_mp_fmt->width; mbus_fmt->height = pix_mp_fmt->height;
Signature · 1 line
2.17.1
Re: [PATCH v2 1/3] media: v4l2-core: cleanup coding style at V4L2 async/fwnode
Hi Mauro,
Quoted 10 lines · Mauro Carvalho Chehab
There are several coding style issues at those definitions, and the previous patchset added even more.
Address the trivial ones by first calling:
./scripts/checkpatch.pl --strict --fix-inline include/media/v4l2-async.h include/media/v4l2-fwnode.h include/media/v4l2-mediabus.h drivers/media/v4l2-core/v4l2-async.c drivers/media/v4l2-core/v4l2-fwnode.cand then manually adjusting the style where needed.
Signed-off-by Mauro Carvalho Chehab <[email protected]>
For patches 1 and 2:
Signature · 2 lines
Sakari Ailus [email protected]
PATCH v2 2/3 media: v4l2-fwnode: cleanup functions that parse endpoints
In reply to Mauro Carvalho ChehabThere is already a typedef for the parse endpoint function. However, instead of using it, it is redefined at the C file (and on one of the function headers).
Replace them by the function typedef, in order to cleanup several related coding style warnings.
--- drivers/media/v4l2-core/v4l2-fwnode.c | 64 ++++++++++++--------------- include/media/v4l2-fwnode.h | 19 ++++---- 2 files changed, 37 insertions(+), 46 deletions(-)
drivers/media/v4l2-core/v4l2-fwnode.c +28 -36
@@ -596,12 +596,10 @@ EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);static int v4l2_async_notifier_fwnode_parse_endpoint (struct device *dev, -struct v4l2_async_notifier *notifier, -struct fwnode_handle *endpoint, -unsigned int asd_struct_size, -int (*parse_endpoint)(struct device *dev, -struct v4l2_fwnode_endpoint *vep, -struct v4l2_async_subdev *asd)) +struct v4l2_async_notifier *notifier, +struct fwnode_handle *endpoint, +unsigned int asd_struct_size, +parse_endpoint_func parse_endpoint) {struct v4l2_fwnode_endpoint vep = { .bus_type =0 };struct v4l2_async_subdev *asd;
@@ -657,13 +655,12 @@ v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev, }static int -__v4l2_async_notifier_parse_fwnode_endpoints (struct device *dev, -struct v4l2_async_notifier *notifier, -size_t asd_struct_size, -unsigned int port,bool has_port, -int (*parse_endpoint)(struct device *dev, -struct v4l2_fwnode_endpoint *vep, -struct v4l2_async_subdev *asd)) +__v4l2_async_notifier_parse_fwnode_ep (struct device *dev, +struct v4l2_async_notifier *notifier, +size_t asd_struct_size, +unsigned int port, +bool has_port, +parse_endpoint_func parse_endpoint) {struct fwnode_handle *fwnode;int ret =0 ;
@@ -708,31 +705,27 @@ __v4l2_async_notifier_parse_fwnode_endpoints(struct device *dev,int v4l2_async_notifier_parse_fwnode_endpoints (struct device *dev, -struct v4l2_async_notifier *notifier, -size_t asd_struct_size, -int (*parse_endpoint)(struct device *dev, -struct v4l2_fwnode_endpoint *vep, -struct v4l2_async_subdev *asd)) +struct v4l2_async_notifier *notifier, +size_t asd_struct_size, +parse_endpoint_func parse_endpoint) { -return __v4l2_async_notifier_parse_fwnode_endpoints (dev, notifier, - asd_struct_size,0 , - false, - parse_endpoint); +return __v4l2_async_notifier_parse_fwnode_ep (dev, notifier, + asd_struct_size,0 , + false, parse_endpoint); }EXPORT_SYMBOL_GPL (v4l2_async_notifier_parse_fwnode_endpoints);int v4l2_async_notifier_parse_fwnode_endpoints_by_port (struct device *dev, -struct v4l2_async_notifier *notifier, -size_t asd_struct_size,unsigned int port, -int (*parse_endpoint)(struct device *dev, -struct v4l2_fwnode_endpoint *vep, -struct v4l2_async_subdev *asd)) +struct v4l2_async_notifier *notifier, +size_t asd_struct_size, +unsigned int port, +parse_endpoint_func parse_endpoint) { -return __v4l2_async_notifier_parse_fwnode_endpoints (dev, notifier, - asd_struct_size, - port, true, - parse_endpoint); +return __v4l2_async_notifier_parse_fwnode_ep (dev, notifier, + asd_struct_size, + port, true, + parse_endpoint); }EXPORT_SYMBOL_GPL (v4l2_async_notifier_parse_fwnode_endpoints_by_port);
@@ -1176,11 +1169,10 @@ int v4l2_async_register_subdev_sensor_common(struct v4l2_subdev *sd)EXPORT_SYMBOL_GPL (v4l2_async_register_subdev_sensor_common);int v4l2_async_register_fwnode_subdev (struct v4l2_subdev *sd, -size_t asd_struct_size, -unsigned int *ports,unsigned int num_ports, -int (*parse_endpoint)(struct device *dev, -struct v4l2_fwnode_endpoint *vep, -struct v4l2_async_subdev *asd)) +size_t asd_struct_size, +unsigned int *ports, +unsigned int num_ports, +parse_endpoint_func parse_endpoint) {struct v4l2_async_notifier *notifier;struct device *dev = sd->dev;
include/media/v4l2-fwnode.h +9 -10
@@ -346,9 +346,10 @@ v4l2_async_notifier_parse_fwnode_endpoints(struct device *dev,*/ int v4l2_async_notifier_parse_fwnode_endpoints_by_port (struct device *dev, -struct v4l2_async_notifier *notifier, -size_t asd_struct_size,unsigned int port, -parse_endpoint_func parse_endpoint); +struct v4l2_async_notifier *notifier, +size_t asd_struct_size, +unsigned int port, +parse_endpoint_func parse_endpoint);/** * v4l2_fwnode_reference_parse_sensor_common - parse common references on
@@ -369,7 +370,7 @@ v4l2_async_notifier_parse_fwnode_endpoints_by_port(struct device *dev,* -EINVAL if property parsing failed */ int v4l2_async_notifier_parse_fwnode_sensor_common (struct device *dev, -struct v4l2_async_notifier *notifier); +struct v4l2_async_notifier *notifier);/** * v4l2_async_register_fwnode_subdev - registers a sub-device to the
@@ -403,11 +404,9 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,*/ int v4l2_async_register_fwnode_subdev (struct v4l2_subdev *sd, -size_t asd_struct_size, -unsigned int *ports, -unsigned int num_ports, -int (*parse_endpoint)(struct device *dev, -struct v4l2_fwnode_endpoint *vep, -struct v4l2_async_subdev *asd)); +size_t asd_struct_size, +unsigned int *ports, +unsigned int num_ports, +parse_endpoint_func parse_endpoint);#endif /* _V4L2_FWNODE_H */
Signature · 1 line
2.17.1
PATCH v2 3/3 media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call
In reply to Mauro Carvalho ChehabThe v4l2_fwnode_reference_parse_int_props() has a big name, causing it to cause coding style warnings. Also, it depends on a const struct embedded indide a function.
Rearrange the logic in order to move the struct declaration out of such function and use it inside this function.
That cleans up some coding style issues.
--- drivers/media/v4l2-core/v4l2-fwnode.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
drivers/media/v4l2-core/v4l2-fwnode.c +12 -11
@@ -1006,6 +1006,12 @@ v4l2_fwnode_reference_get_int_prop(struct fwnode_handle *fwnode,return fwnode; } +struct v4l2_fwnode_int_props { +const char *name; +const char *const *props; +unsigned int nprops; +}; +/* * v4l2_fwnode_reference_parse_int_props - parse references for async * sub-devices
@@ -1032,13 +1038,14 @@ v4l2_fwnode_reference_get_int_prop(struct fwnode_handle *fwnode,static int v4l2_fwnode_reference_parse_int_props (struct device *dev,struct v4l2_async_notifier *notifier, -const char *prop, -const char *const *props, -unsigned int nprops) +const struct v4l2_fwnode_int_props *p) {struct fwnode_handle *fwnode;unsigned int index;int ret; +const char *prop = p->name; +const char *const *props = p->props; +unsigned int nprops = p->nprops; index =0 ; do {
@@ -1093,11 +1100,7 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev, struct v4l2_async_notifier *notifier) {static const char *const led_props[] = {"led" }; -static const struct { -const char *name; -const char *const *props; -unsigned int nprops; - } props[] = { +static const struct v4l2_fwnode_int_props props[] = { {"flash-leds" , led_props,ARRAY_SIZE (led_props) }, {"lens-focus" ,NULL ,0 }, };
@@ -1109,9 +1112,7 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,if (props[i].props &&is_acpi_node (dev_fwnode (dev))) ret =v4l2_fwnode_reference_parse_int_props (dev, notifier, - props[i].name, - props[i].props, - props[i].nprops); + &props[i]);else ret =v4l2_fwnode_reference_parse (dev, notifier, props[i].name);
Signature · 1 line
2.17.1