Hi Mauro,
On 09/24/2018 10:06 AM, Mauro Carvalho Chehab wrote:
Em Mon, 9 Jul 2018 15:39:02 -0700
Steve Longerbeam <slongerbeam@xxxxxxxxx> escreveu:
Generalize v4l2_async_notifier_fwnode_has_async_subdev() to allow
searching for any type of async subdev, not just fwnodes. Rename to
v4l2_async_notifier_has_async_subdev() and pass it an asd pointer.
Signed-off-by: Steve Longerbeam <steve_longerbeam@xxxxxxxxxx>
---
Changes since v5:
- none
Changes since v4:
- none
Changes since v3:
- removed TODO to support asd compare with CUSTOM match type in
asd_equal().
Changes since v2:
- code optimization in asd_equal(), and remove unneeded braces,
suggested by Sakari Ailus.
Changes since v1:
- none
---
drivers/media/v4l2-core/v4l2-async.c | 73 +++++++++++++++++++++---------------
1 file changed, 43 insertions(+), 30 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 2b08d03..0e7e529 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -124,6 +124,31 @@ static struct v4l2_async_subdev *v4l2_async_find_match(
return NULL;
}
+/* Compare two asd's for equivalence */
Please, on comments, instead of "asd" prefer to use what this 3 random
letters mean, e. g.:
asd -> asynchronous subdevice
Ok, I will change the comment to read:
/* Compare two async subdevice descriptors for equivalence */
+static bool asd_equal(struct v4l2_async_subdev *asd_x,
+ struct v4l2_async_subdev *asd_y)
+{
+ if (asd_x->match_type != asd_y->match_type)
+ return false;
+
+ switch (asd_x->match_type) {
+ case V4L2_ASYNC_MATCH_DEVNAME:
+ return strcmp(asd_x->match.device_name,
+ asd_y->match.device_name) == 0;
+ case V4L2_ASYNC_MATCH_I2C:
+ return asd_x->match.i2c.adapter_id ==
+ asd_y->match.i2c.adapter_id &&
+ asd_x->match.i2c.address ==
+ asd_y->match.i2c.address;
+ case V4L2_ASYNC_MATCH_FWNODE:
+ return asd_x->match.fwnode == asd_y->match.fwnode;
+ default:
+ break;
+ }
+
+ return false;
+}
+
/* 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)
@@ -308,29 +333,22 @@ static void v4l2_async_notifier_unbind_all_subdevs(
notifier->parent = NULL;
}
-/* See if an fwnode can be found in a notifier's lists. */
-static bool __v4l2_async_notifier_fwnode_has_async_subdev(
- struct v4l2_async_notifier *notifier, struct fwnode_handle *fwnode)
+/* See if an async sub-device can be found in a notifier's lists. */
+static bool __v4l2_async_notifier_has_async_subdev(
+ struct v4l2_async_notifier *notifier, struct v4l2_async_subdev *asd)
This is a minor issue, but checkpatch complains (with reason)
(with --strict) about the above:
CHECK: Lines should not end with a '('
#63: FILE: drivers/media/v4l2-core/v4l2-async.c:337:
+static bool __v4l2_async_notifier_has_async_subdev(
Better to declare it, instead, as:
static bool
__v4l2_async_notifier_has_async_subdev(struct v4l2_async_notifier *notifier,
struct v4l2_async_subdev *asd)
Similar warnings appear on other places:
CHECK: Lines should not end with a '('
#102: FILE: drivers/media/v4l2-core/v4l2-async.c:362:
+static bool v4l2_async_notifier_has_async_subdev(
CHECK: Lines should not end with a '('
#141: FILE: drivers/media/v4l2-core/v4l2-async.c:410:
+ if (v4l2_async_notifier_has_async_subdev(
Will fix.
Btw, Checkpatch also complains that the author's email is different
than the SOB's one:
WARNING: Missing Signed-off-by: line by nominal patch author 'Steve Longerbeam <slongerbeam@xxxxxxxxx>'
(the some comes with Signed-off-by: Steve Longerbeam <steve_longerbeam@xxxxxxxxxx>)
I suspect that other patches on this series will suffer from the same issue.
Will fix when submitting v7.
<snip>
@@ -392,12 +406,11 @@ static int __v4l2_async_notifier_register(struct v4l2_async_notifier *notifier)
case V4L2_ASYNC_MATCH_CUSTOM:
case V4L2_ASYNC_MATCH_DEVNAME:
case V4L2_ASYNC_MATCH_I2C:
- break;
case V4L2_ASYNC_MATCH_FWNODE:
- if (v4l2_async_notifier_fwnode_has_async_subdev(
- notifier, asd->match.fwnode, i)) {
+ if (v4l2_async_notifier_has_async_subdev(
+ notifier, asd, i)) {
dev_err(dev,
- "fwnode has already been registered or in notifier's subdev list\n");
+ "asd has already been registered or in notifier's subdev list\n");
Please, never use "asd" on messages printed to the user. While someone
may understand it while reading the source code, for a poor use,
"asd" is just a random sequence of 3 characters.
I will change the message to read:
"subdev descriptor already listed in this or other notifiers".
Steve
