On 09/12/2017 03:41 PM, Sakari Ailus wrote:
> Add function v4l2_fwnode_reference_count() for counting external

???? There is no function v4l2_fwnode_reference_count()?!

> 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.

You forgot to remove this outdated paragraph.

> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 69 
> +++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 44ee35f6aad5..a32473f95be1 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -498,6 +498,75 @@ int v4l2_async_notifier_parse_fwnode_endpoints_by_port(
>  }
>  EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);
>  
> +/*
> + * v4l2_fwnode_reference_parse - parse references for async sub-devices
> + * @dev: the device node the properties of which are parsed for references
> + * @notifier: the async notifier where the async subdevs will be added
> + * @prop: the name of the property
> + *
> + * Return: 0 on success
> + *      -ENOENT if no entries were found
> + *      -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)
> +{
> +     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));
> +          index++)
> +             fwnode_handle_put(args.fwnode);
> +
> +     if (!index)
> +             return -ENOENT;
> +
> +     /*
> +      * To-do: handle -ENODATA when "device property: Align return
> +      * codes of acpi_fwnode_get_reference_with_args" is merged.
> +      */
> +     if (ret != -ENOENT && ret != -ENODATA)

So while that patch referenced in the To-do above is not merged yet,
what does fwnode_property_get_reference_args return? ENOENT or ENODATA?
Or ENOENT now and ENODATA later? Or vice versa?

I can't tell based on that information whether this code is correct or not.

The comment needs to explain this a bit better.

> +             return ret;
> +
> +     ret = v4l2_async_notifier_realloc(notifier,
> +                                       notifier->num_subdevs + index);
> +     if (ret)
> +             return ret;
> +
> +     for (index = 0; !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++;
> +     }
> +
> +     return 0;
> +
> +error:
> +     fwnode_handle_put(args.fwnode);
> +     return ret;
> +}
> +
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Sakari Ailus <sakari.ai...@linux.intel.com>");
>  MODULE_AUTHOR("Sylwester Nawrocki <s.nawro...@samsung.com>");
> 

Regards,

        Hans

Reply via email to