On 26/09/17 10:17, Sakari Ailus wrote: > Hi Hans, > > On Tue, Sep 26, 2017 at 10:12:50AM +0200, Hans Verkuil wrote: >> On 26/09/17 00:25, Sakari Ailus wrote: >>> Refactor the V4L2 async framework a little in preparation for async >>> sub-device notifiers. >> >> Perhaps extend this a bit to also state the reason for these changes? > > Well, the true reason is that Laurent felt the following patch was doing > too much, but most of the changes in it are actually tightly > interconnected. > > How about adding: > > This avoids making some structural changes in the patch actually > implementing sub-device notifiers, making that patch easier to review. That will do. I preferred the original version, that way I could see WHY things were done, even though the patch was larger. Regards, Hans > >> >>> >>> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> >> >> Anyway, >> >> Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> > > Thanks! > >> >>> --- >>> drivers/media/v4l2-core/v4l2-async.c | 66 +++++++++++++++++++++++++----------- >>> 1 file changed, 47 insertions(+), 19 deletions(-) >>> >>> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c >>> index 77b9f851bfa9..1d4132305243 100644 >>> --- a/drivers/media/v4l2-core/v4l2-async.c >>> +++ b/drivers/media/v4l2-core/v4l2-async.c >>> @@ -125,12 +125,13 @@ static struct v4l2_async_subdev *v4l2_async_find_match( >>> } >>> >>> static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier, >>> + struct v4l2_device *v4l2_dev, >>> struct v4l2_subdev *sd, >>> struct v4l2_async_subdev *asd) >>> { >>> int ret; >>> >>> - ret = v4l2_device_register_subdev(notifier->v4l2_dev, sd); >>> + ret = v4l2_device_register_subdev(v4l2_dev, sd); >>> if (ret < 0) >>> return ret; >>> >>> @@ -154,6 +155,31 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier, >>> return 0; >>> } >>> >>> +/* Test all async sub-devices in a notifier for a match. */ >>> +static int v4l2_async_notifier_try_all_subdevs( >>> + struct v4l2_async_notifier *notifier) >>> +{ >>> + struct v4l2_device *v4l2_dev = notifier->v4l2_dev; >>> + struct v4l2_subdev *sd, *tmp; >>> + >>> + list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) { >>> + struct v4l2_async_subdev *asd; >>> + int ret; >>> + >>> + asd = v4l2_async_find_match(notifier, sd); >>> + if (!asd) >>> + continue; >>> + >>> + ret = v4l2_async_match_notify(notifier, v4l2_dev, sd, asd); >>> + if (ret < 0) { >>> + mutex_unlock(&list_lock); >>> + return ret; >>> + } >>> + } >>> + >>> + return 0; >>> +} >>> + >>> static void v4l2_async_cleanup(struct v4l2_subdev *sd) >>> { >>> v4l2_device_unregister_subdev(sd); >>> @@ -163,17 +189,15 @@ static void v4l2_async_cleanup(struct v4l2_subdev *sd) >>> sd->dev = NULL; >>> } >>> >>> -int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, >>> - struct v4l2_async_notifier *notifier) >>> +static int __v4l2_async_notifier_register(struct v4l2_async_notifier *notifier) >>> { >>> - struct v4l2_subdev *sd, *tmp; >>> struct v4l2_async_subdev *asd; >>> + int ret; >>> int i; >>> >>> - if (!v4l2_dev || notifier->num_subdevs > V4L2_MAX_SUBDEVS) >>> + if (notifier->num_subdevs > V4L2_MAX_SUBDEVS) >>> return -EINVAL; >>> >>> - notifier->v4l2_dev = v4l2_dev; >>> INIT_LIST_HEAD(¬ifier->waiting); >>> INIT_LIST_HEAD(¬ifier->done); >>> >>> @@ -206,18 +230,10 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, >>> >>> mutex_lock(&list_lock); >>> >>> - list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) { >>> - int ret; >>> - >>> - asd = v4l2_async_find_match(notifier, sd); >>> - if (!asd) >>> - continue; >>> - >>> - ret = v4l2_async_match_notify(notifier, sd, asd); >>> - if (ret < 0) { >>> - mutex_unlock(&list_lock); >>> - return ret; >>> - } >>> + ret = v4l2_async_notifier_try_all_subdevs(notifier); >>> + if (ret) { >>> + mutex_unlock(&list_lock); >>> + return ret; >>> } >>> >>> /* Keep also completed notifiers on the list */ >>> @@ -227,6 +243,17 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, >>> >>> return 0; >>> } >>> + >>> +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, >>> + struct v4l2_async_notifier *notifier) >>> +{ >>> + if (WARN_ON(!v4l2_dev)) >>> + return -EINVAL; >>> + >>> + notifier->v4l2_dev = v4l2_dev; >>> + >>> + return __v4l2_async_notifier_register(notifier); >>> +} >>> EXPORT_SYMBOL(v4l2_async_notifier_register); >>> >>> void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) >>> @@ -303,7 +330,8 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd) >>> struct v4l2_async_subdev *asd = v4l2_async_find_match(notifier, >>> sd); >>> if (asd) { >>> - int ret = v4l2_async_match_notify(notifier, sd, asd); >>> + int ret = v4l2_async_match_notify( >>> + notifier, notifier->v4l2_dev, sd, asd); >>> mutex_unlock(&list_lock); >>> return ret; >>> } >>> >> > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html