On 03/21/18 01:37, Steve Longerbeam wrote:
> 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.
>
> TODO: support asd compare with CUSTOM match type in asd_equal().
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@xxxxxxxxxx>
> ---
> 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 | 76 ++++++++++++++++++++++--------------
> 1 file changed, 46 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 2b08d03..b59bbac 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -124,6 +124,34 @@ static struct v4l2_async_subdev *v4l2_async_find_match(
> return NULL;
> }
>
> +/* Compare two asd's 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;
> + case V4L2_ASYNC_MATCH_CUSTOM:
> + /* TODO */
This TODO suggests that this is needed for some driver(s) and that it
will be implemented later, but it seems more that nobody actually needs
this. If that's the case, then I'd just drop this case altogether.
Or do I misunderstand this comment?
Regards,
Hans
> + return false;
> + 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 +336,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)
> {
> - struct v4l2_async_subdev *asd;
> + struct v4l2_async_subdev *asd_y;
> struct v4l2_subdev *sd;
>
> - list_for_each_entry(asd, ¬ifier->waiting, list) {
> - if (asd->match_type != V4L2_ASYNC_MATCH_FWNODE)
> - continue;
> -
> - if (asd->match.fwnode == fwnode)
> + list_for_each_entry(asd_y, ¬ifier->waiting, list)
> + if (asd_equal(asd, asd_y))
> return true;
> - }
>
> list_for_each_entry(sd, ¬ifier->done, async_list) {
> if (WARN_ON(!sd->asd))
> continue;
>
> - if (sd->asd->match_type != V4L2_ASYNC_MATCH_FWNODE)
> - continue;
> -
> - if (sd->asd->match.fwnode == fwnode)
> + if (asd_equal(asd, sd->asd))
> return true;
> }
>
> @@ -338,32 +359,28 @@ static bool __v4l2_async_notifier_fwnode_has_async_subdev(
> }
>
> /*
> - * Find out whether an async sub-device was set up for an fwnode already or
> + * Find out whether an async sub-device was set up already or
> * whether it exists in a given notifier before @this_index.
> */
> -static bool v4l2_async_notifier_fwnode_has_async_subdev(
> - struct v4l2_async_notifier *notifier, struct fwnode_handle *fwnode,
> +static bool v4l2_async_notifier_has_async_subdev(
> + struct v4l2_async_notifier *notifier, struct v4l2_async_subdev *asd,
> unsigned int this_index)
> {
> unsigned int j;
>
> lockdep_assert_held(&list_lock);
>
> - /* Check that an fwnode is not being added more than once. */
> + /* Check that an asd is not being added more than once. */
> for (j = 0; j < this_index; j++) {
> - struct v4l2_async_subdev *asd = notifier->subdevs[this_index];
> - struct v4l2_async_subdev *other_asd = notifier->subdevs[j];
> + struct v4l2_async_subdev *asd_y = notifier->subdevs[j];
>
> - if (other_asd->match_type == V4L2_ASYNC_MATCH_FWNODE &&
> - asd->match.fwnode ==
> - other_asd->match.fwnode)
> + if (asd_equal(asd, asd_y))
> return true;
> }
>
> - /* Check than an fwnode did not exist in other notifiers. */
> + /* Check that an asd does not exist in other notifiers. */
> list_for_each_entry(notifier, ¬ifier_list, list)
> - if (__v4l2_async_notifier_fwnode_has_async_subdev(
> - notifier, fwnode))
> + if (__v4l2_async_notifier_has_async_subdev(notifier, asd))
> return true;
>
> return false;
> @@ -392,12 +409,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");
> ret = -EEXIST;
> goto err_unlock;
> }
>
