* [PATCH 0/3] Coding style cleanups after the fwnode patchset
@ 2018-10-04 22:13 Mauro Carvalho Chehab
2018-10-04 22:13 ` [PATCH 1/3] media: v4l2-core: cleanup coding style at V4L2 async/fwnode Mauro Carvalho Chehab
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2018-10-04 22:13 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab, Niklas Söderlund, Sakari Ailus,
Steve Longerbeam, Hans Verkuil, Sebastian Reichel
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.
The most irritating stuff there are functions like:
some_very_long_function_that_has_a_very_comprehensive_name(
...);
Functions ending with a "(" without arguments doesn't follow the
right coding style, and it is an heritage of the usage of "indent".
Ok, it sounds that the patches were actually trying to follow an existing
coding style inside it.
As we're about to close the media merge window, and the fwnode patches
already changed a lot of code there, before that becomes an habit to
follow its weird style, let's fix it.
After this series, all we have is the lack of SPDX inside the sources,
and some long lines (with is inevitable without renaming those kAPI
functions).
Btw, I was tempted to rename them, renaming functions like:
v4l2_async_notifier_parse_fwnode_endpoints_by_port
to something like:
v4l2_async_parse_fwnode_ep_by_port
or even:
v4l2_parse_fwnode_ep_by_port
with is probably good enough, but, as this is part of the kAPI, I
opted to keep it as-is - for now.
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 | 190 ++++++++++++++------------
include/media/v4l2-async.h | 12 +-
include/media/v4l2-fwnode.h | 45 +++---
include/media/v4l2-mediabus.h | 32 +++--
5 files changed, 177 insertions(+), 147 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] media: v4l2-core: cleanup coding style at V4L2 async/fwnode
2018-10-04 22:13 [PATCH 0/3] Coding style cleanups after the fwnode patchset Mauro Carvalho Chehab
@ 2018-10-04 22:13 ` Mauro Carvalho Chehab
2018-10-05 7:55 ` Sakari Ailus
2018-10-04 22:13 ` [PATCH 2/3] media: v4l2-fwnode: cleanup functions that parse endpoints Mauro Carvalho Chehab
2018-10-04 22:13 ` [PATCH 3/3] media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call Mauro Carvalho Chehab
2 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2018-10-04 22:13 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab, Sakari Ailus, Niklas Söderlund,
Hans Verkuil, Sebastian Reichel, Steve Longerbeam
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 <mchehab+samsung@kernel.org>
---
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(-)
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 70adbd9a01a2..6fdda745a1da 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -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;
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 6300b599c73d..4e518d5fddd8 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -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;
@@ -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));
return -ENXIO;
}
} else {
@@ -452,8 +452,8 @@ static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,
return rval;
if (vep->bus_type == V4L2_MBUS_UNKNOWN)
- v4l2_fwnode_endpoint_parse_parallel_bus(
- fwnode, vep, V4L2_MBUS_UNKNOWN);
+ 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;
- 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);
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;
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);
else
- ret = v4l2_fwnode_reference_parse(
- dev, notifier, props[i].name);
+ ret = v4l2_fwnode_reference_parse(dev, notifier,
+ props[i].name);
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,
- 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))
{
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;
}
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 89b152f52ef9..1497bda66c3b 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -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
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index b4a49ca27579..21b3f9e5c269 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -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 */
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index df1d552e9df6..66cb746ceeb5 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -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;
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] media: v4l2-fwnode: cleanup functions that parse endpoints
2018-10-04 22:13 [PATCH 0/3] Coding style cleanups after the fwnode patchset Mauro Carvalho Chehab
2018-10-04 22:13 ` [PATCH 1/3] media: v4l2-core: cleanup coding style at V4L2 async/fwnode Mauro Carvalho Chehab
@ 2018-10-04 22:13 ` Mauro Carvalho Chehab
2018-10-05 8:01 ` Sakari Ailus
2018-10-04 22:13 ` [PATCH 3/3] media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call Mauro Carvalho Chehab
2 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2018-10-04 22:13 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab, Sakari Ailus, Niklas Söderlund,
Hans Verkuil, Sebastian Reichel, Steve Longerbeam
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 <mchehab+samsung@kernel.org>
---
drivers/media/v4l2-core/v4l2-fwnode.c | 64 ++++++++++++---------------
include/media/v4l2-fwnode.h | 19 ++++----
2 files changed, 37 insertions(+), 46 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 4e518d5fddd8..a7c2487154a4 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -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;
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index 21b3f9e5c269..6d9d9f1839ac 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -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 */
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call
2018-10-04 22:13 [PATCH 0/3] Coding style cleanups after the fwnode patchset Mauro Carvalho Chehab
2018-10-04 22:13 ` [PATCH 1/3] media: v4l2-core: cleanup coding style at V4L2 async/fwnode Mauro Carvalho Chehab
2018-10-04 22:13 ` [PATCH 2/3] media: v4l2-fwnode: cleanup functions that parse endpoints Mauro Carvalho Chehab
@ 2018-10-04 22:13 ` Mauro Carvalho Chehab
2018-10-05 8:03 ` Sakari Ailus
2 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2018-10-04 22:13 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab, Sakari Ailus, Niklas Söderlund,
Hans Verkuil, Sebastian Reichel, Steve Longerbeam
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 <mchehab+samsung@kernel.org>
---
drivers/media/v4l2-core/v4l2-fwnode.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index a7c2487154a4..e0cd119d6f5c 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -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;
- } props[] = {
+ static const struct v4l2_fwnode_int_props props[] = {
{ "flash-leds", led_props, ARRAY_SIZE(led_props) },
{ "lens-focus", NULL, 0 },
};
- unsigned int i;
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);
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] media: v4l2-core: cleanup coding style at V4L2 async/fwnode
2018-10-04 22:13 ` [PATCH 1/3] media: v4l2-core: cleanup coding style at V4L2 async/fwnode Mauro Carvalho Chehab
@ 2018-10-05 7:55 ` Sakari Ailus
2018-10-05 10:12 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 15+ messages in thread
From: Sakari Ailus @ 2018-10-05 7:55 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Niklas Söderlund, Hans Verkuil, Sebastian Reichel,
Steve Longerbeam
Hi Mauro,
On Thu, Oct 04, 2018 at 06:13:46PM -0400, Mauro Carvalho Chehab wrote:
> 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 <mchehab+samsung@kernel.org>
> ---
> 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(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 70adbd9a01a2..6fdda745a1da 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -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,
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.
> + 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)
It's better here. Below, too.
> {
> 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;
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 6300b599c73d..4e518d5fddd8 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -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;
> @@ -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.
> return -ENXIO;
> }
> } else {
> @@ -452,8 +452,8 @@ static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,
> return rval;
>
> 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.
> + 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;
>
> - 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.
> 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;
>
> 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.
> 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.
> 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,
> - 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.
> {
> 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;
> }
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 89b152f52ef9..1497bda66c3b 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -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
> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> index b4a49ca27579..21b3f9e5c269 100644
> --- a/include/media/v4l2-fwnode.h
> +++ b/include/media/v4l2-fwnode.h
> @@ -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 */
> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> index df1d552e9df6..66cb746ceeb5 100644
> --- a/include/media/v4l2-mediabus.h
> +++ b/include/media/v4l2-mediabus.h
The changes below seem fine to me.
> @@ -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;
> --
> 2.17.1
>
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] media: v4l2-fwnode: cleanup functions that parse endpoints
2018-10-04 22:13 ` [PATCH 2/3] media: v4l2-fwnode: cleanup functions that parse endpoints Mauro Carvalho Chehab
@ 2018-10-05 8:01 ` Sakari Ailus
2018-10-05 9:52 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 15+ messages in thread
From: Sakari Ailus @ 2018-10-05 8:01 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Niklas Söderlund, Hans Verkuil, Sebastian Reichel,
Steve Longerbeam
Hi Mauro,
Feel free to ignore the comments on the first patch regarding the functions
below. There are other issues there though.
On Thu, Oct 04, 2018 at 06:13:47PM -0400, Mauro Carvalho Chehab wrote:
> 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 <mchehab+samsung@kernel.org>
> ---
> drivers/media/v4l2-core/v4l2-fwnode.c | 64 ++++++++++++---------------
> include/media/v4l2-fwnode.h | 19 ++++----
> 2 files changed, 37 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 4e518d5fddd8..a7c2487154a4 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -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)
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.
> {
> - 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;
> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> index 21b3f9e5c269..6d9d9f1839ac 100644
> --- a/include/media/v4l2-fwnode.h
> +++ b/include/media/v4l2-fwnode.h
> @@ -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 */
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call
2018-10-04 22:13 ` [PATCH 3/3] media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call Mauro Carvalho Chehab
@ 2018-10-05 8:03 ` Sakari Ailus
2018-10-05 9:54 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 15+ messages in thread
From: Sakari Ailus @ 2018-10-05 8:03 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Niklas Söderlund, Hans Verkuil, Sebastian Reichel,
Steve Longerbeam
Hi Mauro,
On Thu, Oct 04, 2018 at 06:13:48PM -0400, Mauro Carvalho Chehab wrote:
> 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 <mchehab+samsung@kernel.org>
> ---
> drivers/media/v4l2-core/v4l2-fwnode.c | 25 +++++++++++++------------
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index a7c2487154a4..e0cd119d6f5c 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -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;
> - } 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 <sakari.ailus@linux.intel.com>
>
> 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);
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] media: v4l2-fwnode: cleanup functions that parse endpoints
2018-10-05 8:01 ` Sakari Ailus
@ 2018-10-05 9:52 ` Mauro Carvalho Chehab
2018-10-05 10:08 ` Sakari Ailus
0 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2018-10-05 9:52 UTC (permalink / raw)
To: Sakari Ailus
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Niklas Söderlund, Hans Verkuil, Sebastian Reichel,
Steve Longerbeam
Em Fri, 5 Oct 2018 11:01:18 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:
> Hi Mauro,
>
> Feel free to ignore the comments on the first patch regarding the functions
> below. There are other issues there though.
>
> On Thu, Oct 04, 2018 at 06:13:47PM -0400, Mauro Carvalho Chehab wrote:
> > 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 <mchehab+samsung@kernel.org>
> > ---
> > drivers/media/v4l2-core/v4l2-fwnode.c | 64 ++++++++++++---------------
> > include/media/v4l2-fwnode.h | 19 ++++----
> > 2 files changed, 37 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index 4e518d5fddd8..a7c2487154a4 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -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)
>
> 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.
Thanks,
Mauro
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call
2018-10-05 8:03 ` Sakari Ailus
@ 2018-10-05 9:54 ` Mauro Carvalho Chehab
2018-10-05 10:06 ` Sakari Ailus
0 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2018-10-05 9:54 UTC (permalink / raw)
To: Sakari Ailus
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Niklas Söderlund, Hans Verkuil, Sebastian Reichel,
Steve Longerbeam
Em Fri, 5 Oct 2018 11:03:10 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:
> Hi Mauro,
>
> On Thu, Oct 04, 2018 at 06:13:48PM -0400, Mauro Carvalho Chehab wrote:
> > 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 <mchehab+samsung@kernel.org>
> > ---
> > drivers/media/v4l2-core/v4l2-fwnode.c | 25 +++++++++++++------------
> > 1 file changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index a7c2487154a4..e0cd119d6f5c 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -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;
> > - } 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).
> Apart from that,
>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
> >
> > 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call
2018-10-05 9:54 ` Mauro Carvalho Chehab
@ 2018-10-05 10:06 ` Sakari Ailus
2018-10-05 10:31 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 15+ messages in thread
From: Sakari Ailus @ 2018-10-05 10:06 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Niklas Söderlund, Hans Verkuil, Sebastian Reichel,
Steve Longerbeam
On Fri, Oct 05, 2018 at 06:54:49AM -0300, Mauro Carvalho Chehab wrote:
> Em Fri, 5 Oct 2018 11:03:10 +0300
> Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:
>
> > Hi Mauro,
> >
> > On Thu, Oct 04, 2018 at 06:13:48PM -0400, Mauro Carvalho Chehab wrote:
> > > 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 <mchehab+samsung@kernel.org>
> > > ---
> > > drivers/media/v4l2-core/v4l2-fwnode.c | 25 +++++++++++++------------
> > > 1 file changed, 13 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > index a7c2487154a4..e0cd119d6f5c 100644
> > > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > @@ -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;
> > > - } 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).
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.
See e.g. drivers/media/v4l2-core/v4l2-ctrls.c .
>
> > Apart from that,
> >
> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >
> > >
> > > 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
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] media: v4l2-fwnode: cleanup functions that parse endpoints
2018-10-05 9:52 ` Mauro Carvalho Chehab
@ 2018-10-05 10:08 ` Sakari Ailus
2018-10-05 10:33 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 15+ messages in thread
From: Sakari Ailus @ 2018-10-05 10:08 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Niklas Söderlund, Hans Verkuil, Sebastian Reichel,
Steve Longerbeam
On Fri, Oct 05, 2018 at 06:52:20AM -0300, Mauro Carvalho Chehab wrote:
> Em Fri, 5 Oct 2018 11:01:18 +0300
> Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:
>
> > Hi Mauro,
> >
> > Feel free to ignore the comments on the first patch regarding the functions
> > below. There are other issues there though.
> >
> > On Thu, Oct 04, 2018 at 06:13:47PM -0400, Mauro Carvalho Chehab wrote:
> > > 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 <mchehab+samsung@kernel.org>
> > > ---
> > > drivers/media/v4l2-core/v4l2-fwnode.c | 64 ++++++++++++---------------
> > > include/media/v4l2-fwnode.h | 19 ++++----
> > > 2 files changed, 37 insertions(+), 46 deletions(-)
> > >
> > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > index 4e518d5fddd8..a7c2487154a4 100644
> > > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > @@ -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)
> >
> > 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.
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] media: v4l2-core: cleanup coding style at V4L2 async/fwnode
2018-10-05 7:55 ` Sakari Ailus
@ 2018-10-05 10:12 ` Mauro Carvalho Chehab
2018-10-05 10:22 ` Sakari Ailus
0 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2018-10-05 10:12 UTC (permalink / raw)
To: Sakari Ailus
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Niklas Söderlund, Hans Verkuil, Sebastian Reichel,
Steve Longerbeam
Em Fri, 5 Oct 2018 10:55:58 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:
> Hi Mauro,
>
> On Thu, Oct 04, 2018 at 06:13:46PM -0400, Mauro Carvalho Chehab wrote:
> > 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 <mchehab+samsung@kernel.org>
> > ---
> > 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(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> > index 70adbd9a01a2..6fdda745a1da 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -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,
>
> 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.
Yeah, makes sense.
>
> > + 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)
>
> It's better here. Below, too.
>
> > {
> > 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;
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index 6300b599c73d..4e518d5fddd8 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -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;
> > @@ -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.
>
> > return -ENXIO;
> > }
> > } else {
> > @@ -452,8 +452,8 @@ static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,
> > return rval;
> >
> > 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.
>
> > + 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;
> >
> > - 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.
>
> > 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;
> >
> > 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.
See patch 3.
>
> > 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.
>
> > 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,
> > - 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.
I solved this on patch 2.
>
> > {
> > 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;
> > }
> > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> > index 89b152f52ef9..1497bda66c3b 100644
> > --- a/include/media/v4l2-async.h
> > +++ b/include/media/v4l2-async.h
> > @@ -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
> > diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> > index b4a49ca27579..21b3f9e5c269 100644
> > --- a/include/media/v4l2-fwnode.h
> > +++ b/include/media/v4l2-fwnode.h
> > @@ -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 */
> > diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> > index df1d552e9df6..66cb746ceeb5 100644
> > --- a/include/media/v4l2-mediabus.h
> > +++ b/include/media/v4l2-mediabus.h
>
> The changes below seem fine to me.
>
> > @@ -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;
> > --
> > 2.17.1
> >
>
Thanks,
Mauro
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] media: v4l2-core: cleanup coding style at V4L2 async/fwnode
2018-10-05 10:12 ` Mauro Carvalho Chehab
@ 2018-10-05 10:22 ` Sakari Ailus
0 siblings, 0 replies; 15+ messages in thread
From: Sakari Ailus @ 2018-10-05 10:22 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Niklas Söderlund, Hans Verkuil, Sebastian Reichel,
Steve Longerbeam
Hi Mauro,
On Fri, Oct 05, 2018 at 07:12:02AM -0300, Mauro Carvalho Chehab wrote:
> Em Fri, 5 Oct 2018 10:55:58 +0300
> Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:
...
> > > @@ -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.
> >
> > > return -ENXIO;
> > > }
> > > } else {
> > > @@ -452,8 +452,8 @@ static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,
> > > return rval;
> > >
> > > 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);
I think this is somewhat a matter of taste and I prefer it different. :-)
>
> 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.
Right; V4L2 has a lot of that pattern (also elsewhere) but you'd get told
to fix that if it were in another tree (non-media). I think we agree on
renaming the very long function names; it'll get rid of probably much of
that pattern. I'll submit patches for that later on, possibly including
other improvements to the API. But that'll be after 4.20.
>
> >
> > > + 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;
> > >
> > > - 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.
I also review the patches using Mutt and my terminal window width is set at
80 characters. That's not uncommon I believe.
>
> 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.
That's a valid point, yet more important is that it's not at the same level
than the first line of the statement (function call). So I think we're
discussing matters of somewhat secondary importance.
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call
2018-10-05 10:06 ` Sakari Ailus
@ 2018-10-05 10:31 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2018-10-05 10:31 UTC (permalink / raw)
To: Sakari Ailus
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Niklas Söderlund, Hans Verkuil, Sebastian Reichel,
Steve Longerbeam
Em Fri, 5 Oct 2018 13:06:04 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:
> > > > - 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.
I've seen more than one practice of ordering arguments at the Kernel :-)
Anyway, I kept it there on the version I just sent.
Thanks,
Mauro
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] media: v4l2-fwnode: cleanup functions that parse endpoints
2018-10-05 10:08 ` Sakari Ailus
@ 2018-10-05 10:33 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2018-10-05 10:33 UTC (permalink / raw)
To: Sakari Ailus
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Niklas Söderlund, Hans Verkuil, Sebastian Reichel,
Steve Longerbeam
Em Fri, 5 Oct 2018 13:08:25 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:
> > > 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.
OK! Feel free to rename them when you feel ready. My suggestion is
to do it at the end of a media merging cycle, as makes easier to
avoid conflicts.
I don't care that much about 80 cols. Yet, here it makes a point: we
should be more spartan when naming functions :-)
Thanks,
Mauro
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-10-05 17:32 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-04 22:13 [PATCH 0/3] Coding style cleanups after the fwnode patchset Mauro Carvalho Chehab
2018-10-04 22:13 ` [PATCH 1/3] media: v4l2-core: cleanup coding style at V4L2 async/fwnode Mauro Carvalho Chehab
2018-10-05 7:55 ` Sakari Ailus
2018-10-05 10:12 ` Mauro Carvalho Chehab
2018-10-05 10:22 ` Sakari Ailus
2018-10-04 22:13 ` [PATCH 2/3] media: v4l2-fwnode: cleanup functions that parse endpoints Mauro Carvalho Chehab
2018-10-05 8:01 ` Sakari Ailus
2018-10-05 9:52 ` Mauro Carvalho Chehab
2018-10-05 10:08 ` Sakari Ailus
2018-10-05 10:33 ` Mauro Carvalho Chehab
2018-10-04 22:13 ` [PATCH 3/3] media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call Mauro Carvalho Chehab
2018-10-05 8:03 ` Sakari Ailus
2018-10-05 9:54 ` Mauro Carvalho Chehab
2018-10-05 10:06 ` Sakari Ailus
2018-10-05 10:31 ` Mauro Carvalho Chehab
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.