Coding style cleanups after the fwnode patchset

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.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 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 +1 -0

index 70adbd9a01a2..6fdda745a1da 100644
@@ -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;

drivers/media/v4l2-core/v4l2-async.c +2 -1

+					 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, &notifier->waiting, list) {

drivers/media/v4l2-core/v4l2-async.c +18 -15

 {
 	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(&notifier->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 +5 -4

index 6300b599c73d..4e518d5fddd8 100644
@@ -211,7 +211,7 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
 	if (lanes_used & BIT(clock_lane)) {
 		if (have_clk_lane || !use_default_lane_mapping)
 			pr_warn("duplicated lane %u in clock-lanes, using defaults\n",
-			v);
+				v);
 		use_default_lane_mapping = true;
 	}
 

@@ -265,9 +265,10 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
 			     V4L2_MBUS_FIELD_EVEN_HIGH |	\
 			     V4L2_MBUS_FIELD_EVEN_LOW)
 
-static void v4l2_fwnode_endpoint_parse_parallel_bus(
-	struct fwnode_handle *fwnode, struct v4l2_fwnode_endpoint *vep,
-	enum v4l2_mbus_type bus_type)
+static void
+v4l2_fwnode_endpoint_parse_parallel_bus(struct fwnode_handle *fwnode,
+					struct v4l2_fwnode_endpoint *vep,
+					enum v4l2_mbus_type bus_type)
 {
 	struct v4l2_fwnode_bus_parallel *bus = &vep->bus.parallel;
 	unsigned int flags = 0;

drivers/media/v4l2-core/v4l2-fwnode.c +4 -2

+			v4l2_fwnode_endpoint_parse_parallel_bus(fwnode, vep,
+								V4L2_MBUS_UNKNOWN);
 
 		pr_debug("assuming media bus type %s (%u)\n",
 			 v4l2_fwnode_mbus_type_to_string(vep->bus_type),

@@ -511,8 +511,8 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep)
 }
 EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_free);
 
-int v4l2_fwnode_endpoint_alloc_parse(
-	struct fwnode_handle *fwnode, struct v4l2_fwnode_endpoint *vep)
+int v4l2_fwnode_endpoint_alloc_parse(struct fwnode_handle *fwnode,
+				     struct v4l2_fwnode_endpoint *vep)
 {
 	int rval;
 

drivers/media/v4l2-core/v4l2-fwnode.c +75 -50

 		if (rval < 0) {
 			v4l2_fwnode_endpoint_free(vep);
 			return rval;

@@ -593,12 +594,14 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link)
 }
 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))
+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_fwnode_endpoint vep = { .bus_type = 0 };
 	struct v4l2_async_subdev *asd;

@@ -653,12 +656,14 @@ static int v4l2_async_notifier_fwnode_parse_endpoint(
 	return ret == -ENOTCONN ? 0 : ret;
 }
 
-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))
+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))
 {
 	struct fwnode_handle *fwnode;
 	int ret = 0;

@@ -687,8 +692,11 @@ static int __v4l2_async_notifier_parse_fwnode_endpoints(
 				continue;
 		}
 
-		ret = v4l2_async_notifier_fwnode_parse_endpoint(
-			dev, notifier, fwnode, asd_struct_size, parse_endpoint);
+		ret = v4l2_async_notifier_fwnode_parse_endpoint(dev,
+								notifier,
+								fwnode,
+								asd_struct_size,
+								parse_endpoint);
 		if (ret < 0)
 			break;
 	}

@@ -698,27 +706,33 @@ static int __v4l2_async_notifier_parse_fwnode_endpoints(
 	return ret;
 }
 
-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))
+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))
 {
-	return __v4l2_async_notifier_parse_fwnode_endpoints(
-		dev, notifier, asd_struct_size, 0, false, parse_endpoint);
+	return __v4l2_async_notifier_parse_fwnode_endpoints(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))
+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))
 {
-	return __v4l2_async_notifier_parse_fwnode_endpoints(
-		dev, notifier, asd_struct_size, port, true, parse_endpoint);
+	return __v4l2_async_notifier_parse_fwnode_endpoints(dev, notifier,
+							    asd_struct_size,
+							    port, true,
+							    parse_endpoint);
 }
 EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);
 

@@ -733,17 +747,18 @@ EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);
  *	   -ENOMEM if memory allocation failed
  *	   -EINVAL if property parsing failed
  */
-static int v4l2_fwnode_reference_parse(
-	struct device *dev, struct v4l2_async_notifier *notifier,
-	const char *prop)
+static int v4l2_fwnode_reference_parse(struct device *dev,
+				       struct v4l2_async_notifier *notifier,
+				       const char *prop)
 {
 	struct fwnode_reference_args args;
 	unsigned int index;
 	int ret;
 
 	for (index = 0;
-	     !(ret = fwnode_property_get_reference_args(
-		       dev_fwnode(dev), prop, NULL, 0, index, &args));
+	     !(ret = fwnode_property_get_reference_args(dev_fwnode(dev),
+							prop, NULL, 0,
+							index, &args));
 	     index++)
 		fwnode_handle_put(args.fwnode);
 

@@ -757,13 +772,15 @@ static int v4l2_fwnode_reference_parse(
 	if (ret != -ENOENT && ret != -ENODATA)
 		return ret;
 
-	for (index = 0; !fwnode_property_get_reference_args(
-		     dev_fwnode(dev), prop, NULL, 0, index, &args);
+	for (index = 0;
+	     !fwnode_property_get_reference_args(dev_fwnode(dev), prop, NULL,
+						 0, index, &args);
 	     index++) {
 		struct v4l2_async_subdev *asd;
 
-		asd = v4l2_async_notifier_add_fwnode_subdev(
-			notifier, args.fwnode, sizeof(*asd));
+		asd = v4l2_async_notifier_add_fwnode_subdev(notifier,
+							    args.fwnode,
+							    sizeof(*asd));
 		if (IS_ERR(asd)) {
 			ret = PTR_ERR(asd);
 			/* not an error if asd already exists */

@@ -939,9 +956,12 @@ static int v4l2_fwnode_reference_parse(
  *	   -EINVAL if property parsing otherwise failed
  *	   -ENOMEM if memory allocation failed
  */
-static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(
-	struct fwnode_handle *fwnode, const char *prop, unsigned int index,
-	const char * const *props, unsigned int nprops)
+static struct fwnode_handle *
+v4l2_fwnode_reference_get_int_prop(struct fwnode_handle *fwnode,
+				   const char *prop,
+				   unsigned int index,
+				   const char * const *props,
+				   unsigned int nprops)
 {
 	struct fwnode_reference_args fwnode_args;
 	u64 *args = fwnode_args.args;

@@ -1016,9 +1036,12 @@ static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(
  *	   -EINVAL if property parsing otherwisefailed
  *	   -ENOMEM if memory allocation failed
  */
-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)
+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)
 {
 	struct fwnode_handle *fwnode;
 	unsigned int index;

@@ -1044,9 +1067,12 @@ static int v4l2_fwnode_reference_parse_int_props(
 		index++;
 	} while (1);
 
-	for (index = 0; !IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(
-					 dev_fwnode(dev), prop, index, props,
-					 nprops))); index++) {
+	for (index = 0;
+	     !IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(dev_fwnode(dev),
+								  prop, index,
+								  props,
+								  nprops)));
+	     index++) {
 		struct v4l2_async_subdev *asd;
 
 		asd = v4l2_async_notifier_add_fwnode_subdev(notifier, fwnode,

@@ -1070,8 +1096,8 @@ static int v4l2_fwnode_reference_parse_int_props(
 	return ret;
 }
 
-int v4l2_async_notifier_parse_fwnode_sensor_common(
-	struct device *dev, struct v4l2_async_notifier *notifier)
+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 {

@@ -1088,12 +1114,14 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(
 		int ret;

drivers/media/v4l2-core/v4l2-fwnode.c +0 -2

 		if (ret && ret != -ENOENT) {
 			dev_warn(dev, "parsing property \"%s\" failed (%d)\n",
 				 props[i].name, ret);

@@ -1147,12 +1175,12 @@ 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,

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

Hi Mauro,

Quoted 34 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.c

and then manually adjusting the style where needed.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 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 +1 -0

index 70adbd9a01a2..6fdda745a1da 100644
@@ -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);
 

drivers/media/v4l2-core/v4l2-async.c static LIST_HEAD(subdev_list); · line 94

Quoted 4 lines

drivers/media/v4l2-core/v4l2-async.c +2 -2

-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,  

This looks odd. If you want to rewrap it, then I'd put the stuff before and including the first asterisk on the first line.

Quoted 9 lines

drivers/media/v4l2-core/v4l2-async.c +2 -1

+					 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, &notifier->waiting, list) {

drivers/media/v4l2-core/v4l2-async.c static bool asd_equal(struct v4l2_async_subdev *asd_x, · line 156

Quoted 7 lines

drivers/media/v4l2-core/v4l2-async.c +2 -2

@@ -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)  

It's better here. Below, too.

Quoted 118 lines

drivers/media/v4l2-core/v4l2-async.c +18 -15

 {
 	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(&notifier->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 +5 -4

index 6300b599c73d..4e518d5fddd8 100644
@@ -211,7 +211,7 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
 	if (lanes_used & BIT(clock_lane)) {
 		if (have_clk_lane || !use_default_lane_mapping)
 			pr_warn("duplicated lane %u in clock-lanes, using defaults\n",
-			v);
+				v);
 		use_default_lane_mapping = true;
 	}
 

@@ -265,9 +265,10 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
 			     V4L2_MBUS_FIELD_EVEN_HIGH |	\
 			     V4L2_MBUS_FIELD_EVEN_LOW)
 
-static void v4l2_fwnode_endpoint_parse_parallel_bus(
-	struct fwnode_handle *fwnode, struct v4l2_fwnode_endpoint *vep,
-	enum v4l2_mbus_type bus_type)
+static void
+v4l2_fwnode_endpoint_parse_parallel_bus(struct fwnode_handle *fwnode,
+					struct v4l2_fwnode_endpoint *vep,
+					enum v4l2_mbus_type bus_type)
 {
 	struct v4l2_fwnode_bus_parallel *bus = &vep->bus.parallel;
 	unsigned int flags = 0;

drivers/media/v4l2-core/v4l2-fwnode.c static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode, · line 440

Quoted 6 lines

drivers/media/v4l2-core/v4l2-fwnode.c +1 -2

@@ -436,8 +437,7 @@ static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,
 		if (mbus_type != V4L2_MBUS_UNKNOWN &&
 		    vep->bus_type != mbus_type) {
 			pr_debug("expecting bus type %s\n",
-				 v4l2_fwnode_mbus_type_to_string(
-					 vep->bus_type));
+				 v4l2_fwnode_mbus_type_to_string(vep->bus_type));  

This one's over 80. I preferred what it was. But I have no strong preference here.

Quoted 6 lines

drivers/media/v4l2-core/v4l2-fwnode.c +0 -0

 			return -ENXIO;
 		}
 	} else {

@@ -452,8 +452,8 @@ static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,
 			return rval;
 

drivers/media/v4l2-core/v4l2-fwnode.c static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode, · line 456

Quoted 3 lines

drivers/media/v4l2-core/v4l2-fwnode.c +0 -2

 		if (vep->bus_type == V4L2_MBUS_UNKNOWN)
-			v4l2_fwnode_endpoint_parse_parallel_bus(
-				fwnode, vep, V4L2_MBUS_UNKNOWN);  

This is not uncommon way of aligning function arguments when short of space. It is also not exceeding 80 characters, as the replacement below.

Quoted 21 lines

drivers/media/v4l2-core/v4l2-fwnode.c +4 -2

+			v4l2_fwnode_endpoint_parse_parallel_bus(fwnode, vep,
+								V4L2_MBUS_UNKNOWN);
 
 		pr_debug("assuming media bus type %s (%u)\n",
 			 v4l2_fwnode_mbus_type_to_string(vep->bus_type),

@@ -511,8 +511,8 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep)
 }
 EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_free);
 
-int v4l2_fwnode_endpoint_alloc_parse(
-	struct fwnode_handle *fwnode, struct v4l2_fwnode_endpoint *vep)
+int v4l2_fwnode_endpoint_alloc_parse(struct fwnode_handle *fwnode,
+				     struct v4l2_fwnode_endpoint *vep)
 {
 	int rval;
 

@@ -533,9 +533,10 @@ int v4l2_fwnode_endpoint_alloc_parse(
 
 		vep->nr_of_link_frequencies = rval;
 

drivers/media/v4l2-core/v4l2-fwnode.c int v4l2_fwnode_endpoint_alloc_parse( · line 539

Quoted 7 lines

drivers/media/v4l2-core/v4l2-fwnode.c +4 -3

-		rval = fwnode_property_read_u64_array(
-			fwnode, "link-frequencies", vep->link_frequencies,
-			vep->nr_of_link_frequencies);
+		rval = fwnode_property_read_u64_array(fwnode,
+						      "link-frequencies",
+						      vep->link_frequencies,
+						      vep->nr_of_link_frequencies);  

Over 80 characters.

Quoted 216 lines

drivers/media/v4l2-core/v4l2-fwnode.c +75 -50

 		if (rval < 0) {
 			v4l2_fwnode_endpoint_free(vep);
 			return rval;

@@ -593,12 +594,14 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link)
 }
 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))
+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_fwnode_endpoint vep = { .bus_type = 0 };
 	struct v4l2_async_subdev *asd;

@@ -653,12 +656,14 @@ static int v4l2_async_notifier_fwnode_parse_endpoint(
 	return ret == -ENOTCONN ? 0 : ret;
 }
 
-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))
+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))
 {
 	struct fwnode_handle *fwnode;
 	int ret = 0;

@@ -687,8 +692,11 @@ static int __v4l2_async_notifier_parse_fwnode_endpoints(
 				continue;
 		}
 
-		ret = v4l2_async_notifier_fwnode_parse_endpoint(
-			dev, notifier, fwnode, asd_struct_size, parse_endpoint);
+		ret = v4l2_async_notifier_fwnode_parse_endpoint(dev,
+								notifier,
+								fwnode,
+								asd_struct_size,
+								parse_endpoint);
 		if (ret < 0)
 			break;
 	}

@@ -698,27 +706,33 @@ static int __v4l2_async_notifier_parse_fwnode_endpoints(
 	return ret;
 }
 
-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))
+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))
 {
-	return __v4l2_async_notifier_parse_fwnode_endpoints(
-		dev, notifier, asd_struct_size, 0, false, parse_endpoint);
+	return __v4l2_async_notifier_parse_fwnode_endpoints(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))
+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))
 {
-	return __v4l2_async_notifier_parse_fwnode_endpoints(
-		dev, notifier, asd_struct_size, port, true, parse_endpoint);
+	return __v4l2_async_notifier_parse_fwnode_endpoints(dev, notifier,
+							    asd_struct_size,
+							    port, true,
+							    parse_endpoint);
 }
 EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);
 

@@ -733,17 +747,18 @@ EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);
  *	   -ENOMEM if memory allocation failed
  *	   -EINVAL if property parsing failed
  */
-static int v4l2_fwnode_reference_parse(
-	struct device *dev, struct v4l2_async_notifier *notifier,
-	const char *prop)
+static int v4l2_fwnode_reference_parse(struct device *dev,
+				       struct v4l2_async_notifier *notifier,
+				       const char *prop)
 {
 	struct fwnode_reference_args args;
 	unsigned int index;
 	int ret;
 
 	for (index = 0;
-	     !(ret = fwnode_property_get_reference_args(
-		       dev_fwnode(dev), prop, NULL, 0, index, &args));
+	     !(ret = fwnode_property_get_reference_args(dev_fwnode(dev),
+							prop, NULL, 0,
+							index, &args));
 	     index++)
 		fwnode_handle_put(args.fwnode);
 

@@ -757,13 +772,15 @@ static int v4l2_fwnode_reference_parse(
 	if (ret != -ENOENT && ret != -ENODATA)
 		return ret;
 
-	for (index = 0; !fwnode_property_get_reference_args(
-		     dev_fwnode(dev), prop, NULL, 0, index, &args);
+	for (index = 0;
+	     !fwnode_property_get_reference_args(dev_fwnode(dev), prop, NULL,
+						 0, index, &args);
 	     index++) {
 		struct v4l2_async_subdev *asd;
 
-		asd = v4l2_async_notifier_add_fwnode_subdev(
-			notifier, args.fwnode, sizeof(*asd));
+		asd = v4l2_async_notifier_add_fwnode_subdev(notifier,
+							    args.fwnode,
+							    sizeof(*asd));
 		if (IS_ERR(asd)) {
 			ret = PTR_ERR(asd);
 			/* not an error if asd already exists */

@@ -939,9 +956,12 @@ static int v4l2_fwnode_reference_parse(
  *	   -EINVAL if property parsing otherwise failed
  *	   -ENOMEM if memory allocation failed
  */
-static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(
-	struct fwnode_handle *fwnode, const char *prop, unsigned int index,
-	const char * const *props, unsigned int nprops)
+static struct fwnode_handle *
+v4l2_fwnode_reference_get_int_prop(struct fwnode_handle *fwnode,
+				   const char *prop,
+				   unsigned int index,
+				   const char * const *props,
+				   unsigned int nprops)
 {
 	struct fwnode_reference_args fwnode_args;
 	u64 *args = fwnode_args.args;

@@ -1016,9 +1036,12 @@ static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(
  *	   -EINVAL if property parsing otherwisefailed
  *	   -ENOMEM if memory allocation failed
  */
-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)
+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)
 {
 	struct fwnode_handle *fwnode;
 	unsigned int index;

@@ -1044,9 +1067,12 @@ static int v4l2_fwnode_reference_parse_int_props(
 		index++;
 	} while (1);
 
-	for (index = 0; !IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(
-					 dev_fwnode(dev), prop, index, props,
-					 nprops))); index++) {
+	for (index = 0;
+	     !IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(dev_fwnode(dev),
+								  prop, index,
+								  props,
+								  nprops)));
+	     index++) {
 		struct v4l2_async_subdev *asd;
 
 		asd = v4l2_async_notifier_add_fwnode_subdev(notifier, fwnode,

@@ -1070,8 +1096,8 @@ static int v4l2_fwnode_reference_parse_int_props(
 	return ret;
 }
 
-int v4l2_async_notifier_parse_fwnode_sensor_common(
-	struct device *dev, struct v4l2_async_notifier *notifier)
+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 {

@@ -1088,12 +1114,14 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(
 		int ret;
 

drivers/media/v4l2-core/v4l2-fwnode.c int v4l2_async_notifier_parse_fwnode_sensor_common( · line 1121

Quoted 9 lines

drivers/media/v4l2-core/v4l2-fwnode.c +5 -3

 		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);
+			ret = v4l2_fwnode_reference_parse_int_props(dev,
+								    notifier,
+								    props[i].name,
+								    props[i].props,
+								    props[i].nprops);  

I don't think this kind of changes are improving readability. They push the function arguments out of view on 80-character VTs and split the call to more lines.

drivers/media/v4l2-core/v4l2-fwnode.c int v4l2_async_notifier_parse_fwnode_sensor_common( · line 1124

Quoted 6 lines

drivers/media/v4l2-core/v4l2-fwnode.c +2 -2

 		else
-			ret = v4l2_fwnode_reference_parse(
-				dev, notifier, props[i].name);
+			ret = v4l2_fwnode_reference_parse(dev, notifier,
+							  props[i].name);  

This makes sense though.

Quoted 7 lines

drivers/media/v4l2-core/v4l2-fwnode.c +0 -0

 		if (ret && ret != -ENOENT) {
 			dev_warn(dev, "parsing property \"%s\" failed (%d)\n",
 				 props[i].name, ret);

@@ -1147,12 +1175,12 @@ int v4l2_async_register_subdev_sensor_common(struct v4l2_subdev *sd)
 }
 EXPORT_SYMBOL_GPL(v4l2_async_register_subdev_sensor_common);
 

drivers/media/v4l2-core/v4l2-fwnode.c int v4l2_async_register_subdev_sensor_common(struct v4l2_subdev *sd) · line 1183

Quoted 12 lines

drivers/media/v4l2-core/v4l2-fwnode.c +6 -6

-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))
+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))  

If you wrap it like that, the arguments should be aligned to just right of the opening parentheses. The function argument will go past 80 in that case.

include/media/v4l2-mediabus.h

Quoted 179 lines

drivers/media/v4l2-core/v4l2-fwnode.c +4 -5

 {
 	struct v4l2_async_notifier *notifier;
 	struct device *dev = sd->dev;

@@ -1173,17 +1201,16 @@ int v4l2_async_register_fwnode_subdev(
 	v4l2_async_notifier_init(notifier);
 
 	if (!ports) {
-		ret = v4l2_async_notifier_parse_fwnode_endpoints(
-			dev, notifier, asd_struct_size, parse_endpoint);
+		ret = v4l2_async_notifier_parse_fwnode_endpoints(dev, notifier,
+								 asd_struct_size,
+								 parse_endpoint);
 		if (ret < 0)
 			goto out_cleanup;
 	} else {
 		unsigned int i;
 
 		for (i = 0; i < num_ports; i++) {
-			ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
-				dev, notifier, asd_struct_size,
-				ports[i], parse_endpoint);
+			ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(dev, notifier, asd_struct_size, ports[i], parse_endpoint);
 			if (ret < 0)
 				goto out_cleanup;
 		}

include/media/v4l2-async.h +6 -6

index 89b152f52ef9..1497bda66c3b 100644
@@ -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

index b4a49ca27579..21b3f9e5c269 100644
@@ -71,8 +71,8 @@ struct v4l2_fwnode_bus_parallel {
  * @clock_lane: the number of the clock lane
  */
 struct v4l2_fwnode_bus_mipi_csi1 {
-	bool clock_inv;
-	bool strobe;
+	unsigned char clock_inv:1;
+	unsigned char strobe:1;
 	bool lane_polarity[2];
 	unsigned char data_lane;
 	unsigned char clock_lane;

@@ -203,8 +203,8 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);
  *	   %-EINVAL on parsing failure
  *	   %-ENXIO on mismatching bus types
  */
-int v4l2_fwnode_endpoint_alloc_parse(
-	struct fwnode_handle *fwnode, struct v4l2_fwnode_endpoint *vep);
+int v4l2_fwnode_endpoint_alloc_parse(struct fwnode_handle *fwnode,
+				     struct v4l2_fwnode_endpoint *vep);
 
 /**
  * v4l2_fwnode_parse_link() - parse a link between two endpoints

@@ -236,7 +236,6 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *fwnode,
  */
 void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);
 
-
 /**
  * typedef parse_endpoint_func - Driver's callback function to be called on
  *	each V4L2 fwnode endpoint.

@@ -255,7 +254,6 @@ typedef int (*parse_endpoint_func)(struct device *dev,
 				  struct v4l2_fwnode_endpoint *vep,
 				  struct v4l2_async_subdev *asd);
 
-
 /**
  * v4l2_async_notifier_parse_fwnode_endpoints - Parse V4L2 fwnode endpoints in a
  *						device node

@@ -294,10 +292,11 @@ typedef int (*parse_endpoint_func)(struct device *dev,
  *	   %-EINVAL if graph or endpoint parsing failed
  *	   Other error codes as returned by @parse_endpoint
  */
-int v4l2_async_notifier_parse_fwnode_endpoints(
-	struct device *dev, struct v4l2_async_notifier *notifier,
-	size_t asd_struct_size,
-	parse_endpoint_func parse_endpoint);
+int
+v4l2_async_notifier_parse_fwnode_endpoints(struct device *dev,
+					   struct v4l2_async_notifier *notifier,
+					   size_t asd_struct_size,
+					   parse_endpoint_func parse_endpoint);
 
 /**
  * v4l2_async_notifier_parse_fwnode_endpoints_by_port - Parse V4L2 fwnode

@@ -345,10 +344,11 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
  *	   %-EINVAL if graph or endpoint parsing failed
  *	   Other error codes as returned by @parse_endpoint
  */
-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);
+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);
 
 /**
  * v4l2_fwnode_reference_parse_sensor_common - parse common references on

@@ -368,8 +368,8 @@ int v4l2_async_notifier_parse_fwnode_endpoints_by_port(
  *	   -ENOMEM if memory allocation failed
  *	   -EINVAL if property parsing failed
  */
-int v4l2_async_notifier_parse_fwnode_sensor_common(
-	struct device *dev, struct v4l2_async_notifier *notifier);
+int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
+					struct v4l2_async_notifier *notifier);
 
 /**
  * v4l2_async_register_fwnode_subdev - registers a sub-device to the

@@ -401,11 +401,13 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(
  * An error is returned if the module is no longer loaded on any attempts
  * to register it.
  */
-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));
+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));
 
 #endif /* _V4L2_FWNODE_H */

include/media/v4l2-mediabus.h +0 -0

index df1d552e9df6..66cb746ceeb5 100644

The changes below seem fine to me.

Quoted 69 lines

include/media/v4l2-mediabus.h +16 -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

Quoted 8 lines

include/media/v4l2-mediabus.h +2 -0

+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

Em Fri, 5 Oct 2018 10:55:58 +0300
Sakari Ailus <[email protected]> escreveu:

Quoted 17 lines

drivers/media/v4l2-core/v4l2-fwnode.c static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode, · line 440

Quoted 7 lines

drivers/media/v4l2-core/v4l2-fwnode.c +1 -2

@@ -436,8 +437,7 @@ static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,
 		if (mbus_type != V4L2_MBUS_UNKNOWN &&
 		    vep->bus_type != mbus_type) {
 			pr_debug("expecting bus type %s\n",
-				 v4l2_fwnode_mbus_type_to_string(
-					 vep->bus_type));
+				 v4l2_fwnode_mbus_type_to_string(vep->bus_type));  

This one's over 80. I preferred what it was. But I have no strong preference here.

Quoted 6 lines

drivers/media/v4l2-core/v4l2-fwnode.c +0 -0

 			return -ENXIO;
 		}
 	} else {

@@ -452,8 +452,8 @@ static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,
 			return rval;
 

Quoted 6 lines

drivers/media/v4l2-core/v4l2-fwnode.c static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode, · line 456

Quoted 3 lines

drivers/media/v4l2-core/v4l2-fwnode.c +0 -2

 		if (vep->bus_type == V4L2_MBUS_UNKNOWN)
-			v4l2_fwnode_endpoint_parse_parallel_bus(
-				fwnode, vep, V4L2_MBUS_UNKNOWN);  

This is not uncommon way of aligning function arguments when short of space. It is also not exceeding 80 characters, as the replacement below.

Well, Lindent used to align like that. That's why we see it on lots of code inside media: in the past, people tend to use it to get rid of some checkpatch warnings. Lindent script has long gone (still people sometimes call indent), and now checkpatch evolved, and has a --fix-inplace, with is usually enough to pinpoint where to change (although it does a crap job for more multi-line function args).

As a reviewer, this hurts my eyes. It took me more time to review something like

			v4l2_fwnode_endpoint_parse_parallel_bus(
				fwnode, vep, V4L2_MBUS_UNKNOWN); 

than something like:

			v4l2_fwnode_endpoint_parse_parallel_bus(fwnode, vep,
								V4L2_MBUS_UNKNOWN);

The parenthesis alignment really helps to identify that the second line are arguments.

Btw, if you use checkpatch with --strict, you'll see that this is not the right Kernel coding style. It will complain for both ending a line with an open parenthesis and that the second line is not aligned.

Quoted 22 lines

Quoted 22 lines

drivers/media/v4l2-core/v4l2-fwnode.c +4 -2

+			v4l2_fwnode_endpoint_parse_parallel_bus(fwnode, vep,
+								V4L2_MBUS_UNKNOWN);
 
 		pr_debug("assuming media bus type %s (%u)\n",
 			 v4l2_fwnode_mbus_type_to_string(vep->bus_type),

@@ -511,8 +511,8 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep)
 }
 EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_free);
 
-int v4l2_fwnode_endpoint_alloc_parse(
-	struct fwnode_handle *fwnode, struct v4l2_fwnode_endpoint *vep)
+int v4l2_fwnode_endpoint_alloc_parse(struct fwnode_handle *fwnode,
+				     struct v4l2_fwnode_endpoint *vep)
 {
 	int rval;
 

@@ -533,9 +533,10 @@ int v4l2_fwnode_endpoint_alloc_parse(
 
 		vep->nr_of_link_frequencies = rval;
 

Quoted 9 lines

drivers/media/v4l2-core/v4l2-fwnode.c int v4l2_fwnode_endpoint_alloc_parse( · line 539

Quoted 7 lines

drivers/media/v4l2-core/v4l2-fwnode.c +4 -3

-		rval = fwnode_property_read_u64_array(
-			fwnode, "link-frequencies", vep->link_frequencies,
-			vep->nr_of_link_frequencies);
+		rval = fwnode_property_read_u64_array(fwnode,
+						      "link-frequencies",
+						      vep->link_frequencies,
+						      vep->nr_of_link_frequencies);  

Over 80 characters.

True, but it is better to violate 80-cols (those days, I guess everybody uses a graphical environment), than to not align the arguments.

The 80-cols is there nowadays mostly to warn about code complexity, where multiple indentations are present.

For a reviewer, the parenthesis alignment is a way more relevant, as it allows to immediately notice that the two following lines are arguments of the function, and not a new indentation level.

There 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.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 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 +16 -22

index 4e518d5fddd8..a7c2487154a4 100644
@@ -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

drivers/media/v4l2-core/v4l2-fwnode.c +8 -9

 {
-	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

index 21b3f9e5c269..6d9d9f1839ac 100644
@@ -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 */

Hi Mauro,

Feel free to ignore the comments on the first patch regarding the functions below. There are other issues there though.

Quoted 77 lines · Mauro Carvalho Chehab

There 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.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 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 +16 -22

index 4e518d5fddd8..a7c2487154a4 100644
@@ -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);
 

drivers/media/v4l2-core/v4l2-fwnode.c __v4l2_async_notifier_parse_fwnode_endpoints(struct device *dev, · line 723

Quoted 11 lines

drivers/media/v4l2-core/v4l2-fwnode.c +4 -5

 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)  

This is still over 80 here. I think we could think of abbreviating what's in the function name, not limiting to the endpoint. I think I'd prefer to leave that for 4.21 as there's not much time anymore.

Em Fri, 5 Oct 2018 11:01:18 +0300
Sakari Ailus <[email protected]> escreveu:

Quoted 83 lines

Hi Mauro,

Feel free to ignore the comments on the first patch regarding the functions below. There are other issues there though.

Quoted 77 lines · Mauro Carvalho Chehab

There 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.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 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 +16 -22

index 4e518d5fddd8..a7c2487154a4 100644
@@ -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);
 

Quoted 15 lines

drivers/media/v4l2-core/v4l2-fwnode.c __v4l2_async_notifier_parse_fwnode_endpoints(struct device *dev, · line 723

Quoted 11 lines

drivers/media/v4l2-core/v4l2-fwnode.c +4 -5

 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)  

This is still over 80 here. I think we could think of abbreviating what's in the function name, not limiting to the endpoint. I think I'd prefer to leave that for 4.21 as there's not much time anymore.

Yes, I know. Renaming the function is the only way to get rid of those remaining warnings. If you're ok with renaming, IMHO it is best do do it right now, as we are already churning a lot of fwnode-related code, avoiding the need of touching it again for 4.21.

Quoted 8 lines

Quoted 3 lines

This is still over 80 here. I think we could think of abbreviating what's in the function name, not limiting to the endpoint. I think I'd prefer to leave that for 4.21 as there's not much time anymore.

Yes, I know. Renaming the function is the only way to get rid of those remaining warnings. If you're ok with renaming, IMHO it is best do do it right now, as we are already churning a lot of fwnode-related code, avoiding the need of touching it again for 4.21.

This will presumably continue in v4.21 (or later). As noted in the cover page of the fwnode patchset:

This patchset does not address remaining issues such as supporting setting defaults for e.g. bridge drivers with multiple ports, but with Steve Longerbeam's patchset we're much closer to that goal.

The 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.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 drivers/media/v4l2-core/v4l2-fwnode.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

drivers/media/v4l2-core/v4l2-fwnode.c +11 -3

index a7c2487154a4..e0cd119d6f5c 100644
@@ -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 {

@@ -1092,16 +1099,12 @@ v4l2_fwnode_reference_parse_int_props(struct device *dev,
 int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
 						   struct v4l2_async_notifier *notifier)
 {
+	unsigned int i;
 	static const char * const led_props[] = { "led" };

drivers/media/v4l2-core/v4l2-fwnode.c +1 -3

 	for (i = 0; i < ARRAY_SIZE(props); i++) {
 		int ret;

@@ -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);

Hi Mauro,

Quoted 59 lines · Mauro Carvalho Chehab

The 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.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 drivers/media/v4l2-core/v4l2-fwnode.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

drivers/media/v4l2-core/v4l2-fwnode.c +11 -7

index a7c2487154a4..e0cd119d6f5c 100644
@@ -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 {

@@ -1092,16 +1099,12 @@ v4l2_fwnode_reference_parse_int_props(struct device *dev,
 int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
 						   struct v4l2_async_notifier *notifier)
 {
+	unsigned int i;
 	static const char * const led_props[] = { "led" };
-	static const struct {
-		const char *name;
-		const char * const *props;
-		unsigned int nprops;

drivers/media/v4l2-core/v4l2-fwnode.c v4l2_fwnode_reference_parse_int_props(struct device *dev, · line 1104

Quoted 6 lines

drivers/media/v4l2-core/v4l2-fwnode.c +1 -2

-	} props[] = {
+	static const struct v4l2_fwnode_int_props props[] = {
 		{ "flash-leds", led_props, ARRAY_SIZE(led_props) },
 		{ "lens-focus", NULL, 0 },
 	};
-	unsigned int i;  

I'd like to keep this here.

Apart from that,

Acked-by Sakari Ailus <[email protected]>

Quoted 1 line

drivers/media/v4l2-core/v4l2-fwnode.c int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev, · line 1118

Quoted 13 lines

drivers/media/v4l2-core/v4l2-fwnode.c +1 -3

 	for (i = 0; i < ARRAY_SIZE(props); i++) {
 		int ret;

@@ -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);  

Em Fri, 5 Oct 2018 11:03:10 +0300
Sakari Ailus <[email protected]> escreveu:

Quoted 62 lines

Hi Mauro,

Quoted 59 lines · Mauro Carvalho Chehab

The 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.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 drivers/media/v4l2-core/v4l2-fwnode.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

drivers/media/v4l2-core/v4l2-fwnode.c +11 -7

index a7c2487154a4..e0cd119d6f5c 100644
@@ -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 {

@@ -1092,16 +1099,12 @@ v4l2_fwnode_reference_parse_int_props(struct device *dev,
 int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
 						   struct v4l2_async_notifier *notifier)
 {
+	unsigned int i;
 	static const char * const led_props[] = { "led" };
-	static const struct {
-		const char *name;
-		const char * const *props;
-		unsigned int nprops;

Quoted 8 lines

drivers/media/v4l2-core/v4l2-fwnode.c v4l2_fwnode_reference_parse_int_props(struct device *dev, · line 1104

Quoted 6 lines

drivers/media/v4l2-core/v4l2-fwnode.c +1 -2

-	} props[] = {
+	static const struct v4l2_fwnode_int_props props[] = {
 		{ "flash-leds", led_props, ARRAY_SIZE(led_props) },
 		{ "lens-focus", NULL, 0 },
 	};
-	unsigned int i;  

I'd like to keep this here.

Why? IMHO, it makes harder to read (yet, if you insist, I'm ok with both ways).

Quoted 6 lines

Apart from that,

Acked-by Sakari Ailus <[email protected]>

Quoted 1 line

Quoted 14 lines

drivers/media/v4l2-core/v4l2-fwnode.c int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev, · line 1118

Quoted 13 lines

drivers/media/v4l2-core/v4l2-fwnode.c +1 -3

 	for (i = 0; i < ARRAY_SIZE(props); i++) {
 		int ret;

@@ -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);  

Thanks,
Mauro

Quoted 6 lines

Quoted 3 lines

drivers/media/v4l2-core/v4l2-fwnode.c v4l2_fwnode_reference_parse_int_props(struct device *dev, · line 1104

Quoted 1 line

drivers/media/v4l2-core/v4l2-fwnode.c +0 -1

-	unsigned int i;    

I'd like to keep this here.

Why? IMHO, it makes harder to read (yet, if you insist, I'm ok with both ways).

Generally loop, temporary, return etc. variables are nice to declare as last. That is the practice in this file and generally in kernel code, albeit with variable degree of application.