Re: [PATCH v10 17/24] v4l: fwnode: Add a helper function for parsing generic references
From: Hans Verkuil <hidden>
Date: 2017-09-11 09:14:03
Also in:
linux-acpi, linux-media
On 09/11/2017 10:00 AM, Sakari Ailus wrote:
Add function v4l2_fwnode_reference_count() for counting external references and v4l2_fwnode_reference_parse() for parsing them as async sub-devices. This can be done on e.g. flash or lens async sub-devices that are not part of but are associated with a sensor. struct v4l2_async_notifier.max_subdevs field is added to contain the maximum number of sub-devices in a notifier to reflect the memory allocated for the subdevs array.
This paragraph appears to be out-of-date.
quoted hunk ↗ jump to hunk
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- drivers/media/v4l2-core/v4l2-fwnode.c | 47 +++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c index d978f2d714ca..4821c4989119 100644 --- a/drivers/media/v4l2-core/v4l2-fwnode.c +++ b/drivers/media/v4l2-core/v4l2-fwnode.c@@ -449,6 +449,53 @@ int v4l2_async_notifier_parse_fwnode_endpoints( } EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints); +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 = 0; + int ret; + + for (; !fwnode_property_get_reference_args( + dev_fwnode(dev), prop, NULL, 0, index, &args); index++) + fwnode_handle_put(args.fwnode); +
If nothing is found (i.e. index == 0), shouldn't you just return here?
+ ret = v4l2_async_notifier_realloc(notifier, + notifier->num_subdevs + index); + if (ret) + return -ENOMEM; + + for (ret = -ENOENT, index = 0;
There is no reason for the 'ret = -ENOENT' to be in the for(), just set it before the 'for' statement.
+ !fwnode_property_get_reference_args(
+ dev_fwnode(dev), prop, NULL, 0, index, &args);
+ index++) {
+ struct v4l2_async_subdev *asd;
+
+ if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
+ ret = -EINVAL;
+ goto error;
+ }
+
+ asd = kzalloc(sizeof(*asd), GFP_KERNEL);
+ if (!asd) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ notifier->subdevs[notifier->num_subdevs] = asd;
+ asd->match.fwnode.fwnode = args.fwnode;
+ asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
+ notifier->num_subdevs++;
+ }If the loop doesn't find anything, then it still returns 0, not -ENOENT. So why set ret to ENOENT? Something weird going on here. I think you should also add a comment explaining this function.
+
+ return 0;
+
+error:
+ fwnode_handle_put(args.fwnode);
+ return ret;
+}
+
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Sakari Ailus [off-list ref]");
MODULE_AUTHOR("Sylwester Nawrocki [off-list ref]");Regards, Hans