Re: [PATCH v13 05/25] v4l: fwnode: Support generic parsing of graph endpoints in a device
On Tue, Sep 19, 2017 at 03:34:00PM +0300, Laurent Pinchart wrote: > Hi Sakari, > > On Tuesday, 19 September 2017 15:11:32 EEST Sakari Ailus wrote: > > On Tue, Sep 19, 2017 at 02:35:01PM +0300, Laurent Pinchart wrote: > > > On Friday, 15 September 2017 17:17:04 EEST Sakari Ailus wrote: > > >> Add two functions for parsing devices graph endpoints: > > >> v4l2_async_notifier_parse_fwnode_endpoints and > > >> v4l2_async_notifier_parse_fwnode_endpoints_by_port. The former iterates > > >> over all endpoints whereas the latter only iterates over the endpoints > > >> in a given port. > > >> > > >> The former is mostly useful for existing drivers that currently > > >> implement the iteration over all the endpoints themselves whereas the > > >> latter is especially intended for devices with both sinks and sources: > > >> async sub-devices for external devices connected to the device's sources > > >> will have already been set up, or they are part of the master device. > > > > > > Did you mean s/or they/as they/ ? > > > > No. There are two options here: either the sub-devices a sub-device is > > connected to (through a graph endpoint) is instantiated through the async > > framework *or* through the master device driver. But not by both of them at > > the same time. > > The message is then contradicting itself: > > "async sub-devices for external devices connected to the device's sources > will > have already been set up, or they are part of the master device." > > They refers to "async sub-devices". If they're part of the master device, > they're not async sub-devices. Ah, now I see what you mean. I'll replace "they" with "the external sub-devices". The paragraph becomes: The former is mostly useful for existing drivers that currently implement the iteration over all the endpoints themselves whereas the latter is especially intended for devices with both sinks and sources: async sub-devices for external devices connected to the device's sources will have already been set up, or the external sub-devices are part of the master device. How about that? -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi
Re: [PATCH v13 05/25] v4l: fwnode: Support generic parsing of graph endpoints in a device
Hi Sakari, On Tuesday, 19 September 2017 15:11:32 EEST Sakari Ailus wrote: > On Tue, Sep 19, 2017 at 02:35:01PM +0300, Laurent Pinchart wrote: > > On Friday, 15 September 2017 17:17:04 EEST Sakari Ailus wrote: > >> Add two functions for parsing devices graph endpoints: > >> v4l2_async_notifier_parse_fwnode_endpoints and > >> v4l2_async_notifier_parse_fwnode_endpoints_by_port. The former iterates > >> over all endpoints whereas the latter only iterates over the endpoints > >> in a given port. > >> > >> The former is mostly useful for existing drivers that currently > >> implement the iteration over all the endpoints themselves whereas the > >> latter is especially intended for devices with both sinks and sources: > >> async sub-devices for external devices connected to the device's sources > >> will have already been set up, or they are part of the master device. > > > > Did you mean s/or they/as they/ ? > > No. There are two options here: either the sub-devices a sub-device is > connected to (through a graph endpoint) is instantiated through the async > framework *or* through the master device driver. But not by both of them at > the same time. The message is then contradicting itself: "async sub-devices for external devices connected to the device's sources will have already been set up, or they are part of the master device." They refers to "async sub-devices". If they're part of the master device, they're not async sub-devices. -- Regards, Laurent Pinchart
Re: [PATCH v13 05/25] v4l: fwnode: Support generic parsing of graph endpoints in a device
Hi Laurent,
On Tue, Sep 19, 2017 at 02:35:01PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
Thanks for the review!
>
> On Friday, 15 September 2017 17:17:04 EEST Sakari Ailus wrote:
> > Add two functions for parsing devices graph endpoints:
> > v4l2_async_notifier_parse_fwnode_endpoints and
> > v4l2_async_notifier_parse_fwnode_endpoints_by_port. The former iterates
> > over all endpoints whereas the latter only iterates over the endpoints in
> > a given port.
> >
> > The former is mostly useful for existing drivers that currently implement
> > the iteration over all the endpoints themselves whereas the latter is
> > especially intended for devices with both sinks and sources: async
> > sub-devices for external devices connected to the device's sources will
> > have already been set up, or they are part of the master device.
>
> Did you mean s/or they/as they/ ?
No. There are two options here: either the sub-devices a sub-device is
connected to (through a graph endpoint) is instantiated through the async
framework *or* through the master device driver. But not by both of them at
the same time.
>
> > Signed-off-by: Sakari Ailus
> > ---
> > drivers/media/v4l2-core/v4l2-async.c | 30 ++
> > drivers/media/v4l2-core/v4l2-fwnode.c | 185 +++
> > include/media/v4l2-async.h| 24 -
> > include/media/v4l2-fwnode.h | 117 +
> > 4 files changed, 354 insertions(+), 2 deletions(-)
>
> [snip]
>
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> > b/drivers/media/v4l2-core/v4l2-fwnode.c index 706f9e7b90f1..44ee35f6aad5
> > 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
>
> [snip]
>
> > +static int v4l2_async_notifier_fwnode_parse_endpoint(
> > + struct device *dev, struct v4l2_async_notifier *notifier,
> > + struct fwnode_handle *endpoint, unsigned int asd_struct_size,
> > + int (*parse_endpoint)(struct device *dev,
> > + struct v4l2_fwnode_endpoint *vep,
> > + struct v4l2_async_subdev *asd))
> > +{
> > + struct v4l2_async_subdev *asd;
> > + struct v4l2_fwnode_endpoint *vep;
> > + int ret = 0;
> > +
> > + asd = kzalloc(asd_struct_size, GFP_KERNEL);
> > + if (!asd)
> > + return -ENOMEM;
> > +
> > + asd->match.fwnode.fwnode =
> > + fwnode_graph_get_remote_port_parent(endpoint);
> > + if (!asd->match.fwnode.fwnode) {
> > + dev_warn(dev, "bad remote port parent\n");
> > + ret = -EINVAL;
> > + goto out_err;
> > + }
> > +
> > + /* Ignore endpoints the parsing of which failed. */
>
> I think this comment is outdated.
Hmm. Yes. I'll remove it.
>
> > + vep = v4l2_fwnode_endpoint_alloc_parse(endpoint);
> > + if (IS_ERR(vep)) {
> > + ret = PTR_ERR(vep);
> > + dev_warn(dev, "unable to parse V4L2 fwnode endpoint (%d)\n",
> > +ret);
> > + goto out_err;
> > + }
> > +
> > + ret = parse_endpoint ? parse_endpoint(dev, vep, asd) : 0;
> > + if (ret == -ENOTCONN)
> > + dev_dbg(dev, "ignoring endpoint %u,%u\n", vep->base.port,
> > + vep->base.id);
>
> How about "ignoring port@%u/endpoint@%u\n" ? It would make the message more
> explicit.
Works for me.
>
> > + else if (ret < 0)
> > + dev_warn(dev, "driver could not parse endpoint %u,%u (%d)\n",
>
> Same here.
>
> > +vep->base.port, vep->base.id, ret);
> > + v4l2_fwnode_endpoint_free(vep);
> > + if (ret < 0)
> > + goto out_err;
> > +
> > + asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
>
> I'd move this line right before setting asd->match.fwnode.fwnode.
I'll move it.
>
> > + notifier->subdevs[notifier->num_subdevs] = asd;
> > + notifier->num_subdevs++;
> > +
> > + return 0;
> > +
> > +out_err:
> > + fwnode_handle_put(asd->match.fwnode.fwnode);
> > + kfree(asd);
> > +
> > + return ret == -ENOTCONN ? 0 : ret;
> > +}
> > +
> > +static int __v4l2_async_notifier_parse_fwnode_endpoints(
> > + struct device *dev, struct v4l2_async_notifier *notifier,
> > + size_t asd_struct_size, unsigned int port, bool has_port,
> > + int (*parse_endpoint)(struct device *dev,
> > + struct v4l2_fwnode_endpoint *vep,
> > + struct v4l2_async_subdev *asd))
> > +{
> > + struct fwnode_handle *fwnode = NULL;
> > + unsigned int max_subdevs = notifier->max_subdevs;
> > + int ret;
> > +
> > + if (WARN_ON(asd_struct_size < sizeof(struct v4l2_async_subdev)))
> > + return -EINVAL;
> > +
> > + for (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(
> > +dev_fwnode(dev), fwnode)); ) {
> > + if (!fwnode_device_is_available(
> > + fwnode_graph_get_port_parent(fwnode)))
>
> Doesn't fwnode_graph_get_port_parent() increment the refcount on the
Re: [PATCH v13 05/25] v4l: fwnode: Support generic parsing of graph endpoints in a device
Hi Laurent, On Tue, Sep 19, 2017 at 12:30:34PM +0300, Laurent Pinchart wrote: > Hi Hans, > > On Tuesday, 19 September 2017 11:40:14 EEST Hans Verkuil wrote: > > On 09/19/2017 10:20 AM, Sakari Ailus wrote: > > > On Tue, Sep 19, 2017 at 10:03:27AM +0200, Hans Verkuil wrote: > > >> On 09/15/2017 04:17 PM, Sakari Ailus wrote: > > >>> Add two functions for parsing devices graph endpoints: > > >>> v4l2_async_notifier_parse_fwnode_endpoints and > > >>> v4l2_async_notifier_parse_fwnode_endpoints_by_port. The former iterates > > >>> over all endpoints whereas the latter only iterates over the endpoints > > >>> in a given port. > > >>> > > >>> The former is mostly useful for existing drivers that currently > > >>> implement the iteration over all the endpoints themselves whereas the > > >>> latter is especially intended for devices with both sinks and sources: > > >>> async sub-devices for external devices connected to the device's sources > > >>> will have already been set up, or they are part of the master device. > > >>> > > >>> Signed-off-by: Sakari Ailus > > >>> --- > > >>> > > >>> drivers/media/v4l2-core/v4l2-async.c | 30 ++ > > >>> drivers/media/v4l2-core/v4l2-fwnode.c | 185 +++ > > >>> include/media/v4l2-async.h| 24 - > > >>> include/media/v4l2-fwnode.h | 117 + > > >>> 4 files changed, 354 insertions(+), 2 deletions(-) > > [snip] > > > >>> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h > > >>> index 68eb22ba571b..83afac48ea6b 100644 > > >>> --- a/include/media/v4l2-fwnode.h > > >>> +++ b/include/media/v4l2-fwnode.h > > >>> @@ -25,6 +25,8 @@ > > >>> > > >>> #include > > >>> > > >>> struct fwnode_handle; > > >>> +struct v4l2_async_notifier; > > >>> +struct v4l2_async_subdev; > > >>> > > >>> #define V4L2_FWNODE_CSI2_MAX_DATA_LANES4 > > >>> > > >>> @@ -201,4 +203,119 @@ int v4l2_fwnode_parse_link(struct fwnode_handle > > >>> *fwnode, > > >>> */ > > >>> void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link); > > >>> > > >>> +/** > > >>> + * v4l2_async_notifier_parse_fwnode_endpoints - Parse V4L2 fwnode > > >>> endpoints in a > > >>> + * device node > > >>> + * @dev: the device the endpoints of which are to be parsed > > >>> + * @notifier: notifier for @dev > > >>> + * @asd_struct_size: size of the driver's async sub-device struct, > > >>> including > > >>> + * sizeof(struct v4l2_async_subdev). The &struct > > >>> + * v4l2_async_subdev shall be the first member of > > >>> + * the driver's async sub-device struct, i.e. both > > >>> + * begin at the same memory address. > > >>> + * @parse_endpoint: Driver's callback function called on each V4L2 > > >>> fwnode > > >>> + * endpoint. Optional. > > >>> + * Return: %0 on success > > >>> + * %-ENOTCONN if the endpoint is to be skipped > > >>> but this > > >>> + *should not be considered as an > > >>> error > > >>> + * %-EINVAL if the endpoint configuration is > > >>> invalid > > >>> + * > > >>> + * Parse the fwnode endpoints of the @dev device and populate the async > > >>> sub- > > >>> + * devices array of the notifier. The @parse_endpoint callback function > > >>> is > > >>> + * called for each endpoint with the corresponding async sub-device > > >>> pointer to > > >>> + * let the caller initialize the driver-specific part of the async sub- > > >>> device > > >>> + * structure. > > >>> + * > > >>> + * The notifier memory shall be zeroed before this function is called > > >>> on the > > >>> + * notifier. > > >>> + * > > >>> + * This function may not be called on a registered notifier and may be > > >>> called on > > >>> + * a notifier only once. > > >>> + * > > >>> + * Do not change the notifier's subdevs array, take references to the > > >>> subdevs > > >>> + * array itself or change the notifier's num_subdevs field. This is > > >>> because this > > >>> + * function allocates and reallocates the subdevs array based on > > >>> parsing > > >>> + * endpoints. > > >>> + * > > >>> + * The @struct v4l2_fwnode_endpoint passed to the callback function > > >>> + * @parse_endpoint is released once the function is finished. If there > > >>> is a need > > >>> + * to retain that configuration, the user needs to allocate memory for > > >>> it. > > >>> + * > > >>> + * Any notifier populated using this function must be released with a > > >>> call to > > >>> + * v4l2_async_notifier_release() after it has been unregistered and the > > >>> async > > >>> + * sub-devices are no longer in use, even if the function returned an > > >>> error. > > >>> + * > > >>> + * Return: %0 on success, including when no async sub-devices are found > > >>> + *%-ENOMEM if memory allocation failed > > >>> + *%-EINVAL if graph or end
Re: [PATCH v13 05/25] v4l: fwnode: Support generic parsing of graph endpoints in a device
Hi Sakari,
Thank you for the patch.
On Friday, 15 September 2017 17:17:04 EEST Sakari Ailus wrote:
> Add two functions for parsing devices graph endpoints:
> v4l2_async_notifier_parse_fwnode_endpoints and
> v4l2_async_notifier_parse_fwnode_endpoints_by_port. The former iterates
> over all endpoints whereas the latter only iterates over the endpoints in
> a given port.
>
> The former is mostly useful for existing drivers that currently implement
> the iteration over all the endpoints themselves whereas the latter is
> especially intended for devices with both sinks and sources: async
> sub-devices for external devices connected to the device's sources will
> have already been set up, or they are part of the master device.
Did you mean s/or they/as they/ ?
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/v4l2-core/v4l2-async.c | 30 ++
> drivers/media/v4l2-core/v4l2-fwnode.c | 185 +++
> include/media/v4l2-async.h| 24 -
> include/media/v4l2-fwnode.h | 117 +
> 4 files changed, 354 insertions(+), 2 deletions(-)
[snip]
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> b/drivers/media/v4l2-core/v4l2-fwnode.c index 706f9e7b90f1..44ee35f6aad5
> 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
[snip]
> +static int v4l2_async_notifier_fwnode_parse_endpoint(
> + struct device *dev, struct v4l2_async_notifier *notifier,
> + struct fwnode_handle *endpoint, unsigned int asd_struct_size,
> + int (*parse_endpoint)(struct device *dev,
> + struct v4l2_fwnode_endpoint *vep,
> + struct v4l2_async_subdev *asd))
> +{
> + struct v4l2_async_subdev *asd;
> + struct v4l2_fwnode_endpoint *vep;
> + int ret = 0;
> +
> + asd = kzalloc(asd_struct_size, GFP_KERNEL);
> + if (!asd)
> + return -ENOMEM;
> +
> + asd->match.fwnode.fwnode =
> + fwnode_graph_get_remote_port_parent(endpoint);
> + if (!asd->match.fwnode.fwnode) {
> + dev_warn(dev, "bad remote port parent\n");
> + ret = -EINVAL;
> + goto out_err;
> + }
> +
> + /* Ignore endpoints the parsing of which failed. */
I think this comment is outdated.
> + vep = v4l2_fwnode_endpoint_alloc_parse(endpoint);
> + if (IS_ERR(vep)) {
> + ret = PTR_ERR(vep);
> + dev_warn(dev, "unable to parse V4L2 fwnode endpoint (%d)\n",
> + ret);
> + goto out_err;
> + }
> +
> + ret = parse_endpoint ? parse_endpoint(dev, vep, asd) : 0;
> + if (ret == -ENOTCONN)
> + dev_dbg(dev, "ignoring endpoint %u,%u\n", vep->base.port,
> + vep->base.id);
How about "ignoring port@%u/endpoint@%u\n" ? It would make the message more
explicit.
> + else if (ret < 0)
> + dev_warn(dev, "driver could not parse endpoint %u,%u (%d)\n",
Same here.
> + vep->base.port, vep->base.id, ret);
> + v4l2_fwnode_endpoint_free(vep);
> + if (ret < 0)
> + goto out_err;
> +
> + asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
I'd move this line right before setting asd->match.fwnode.fwnode.
> + notifier->subdevs[notifier->num_subdevs] = asd;
> + notifier->num_subdevs++;
> +
> + return 0;
> +
> +out_err:
> + fwnode_handle_put(asd->match.fwnode.fwnode);
> + kfree(asd);
> +
> + return ret == -ENOTCONN ? 0 : ret;
> +}
> +
> +static int __v4l2_async_notifier_parse_fwnode_endpoints(
> + struct device *dev, struct v4l2_async_notifier *notifier,
> + size_t asd_struct_size, unsigned int port, bool has_port,
> + int (*parse_endpoint)(struct device *dev,
> + struct v4l2_fwnode_endpoint *vep,
> + struct v4l2_async_subdev *asd))
> +{
> + struct fwnode_handle *fwnode = NULL;
> + unsigned int max_subdevs = notifier->max_subdevs;
> + int ret;
> +
> + if (WARN_ON(asd_struct_size < sizeof(struct v4l2_async_subdev)))
> + return -EINVAL;
> +
> + for (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(
> + dev_fwnode(dev), fwnode)); ) {
> + if (!fwnode_device_is_available(
> + fwnode_graph_get_port_parent(fwnode)))
Doesn't fwnode_graph_get_port_parent() increment the refcount on the parent,
which you should then release ?
> + continue;
> +
> + if (has_port) {
> + struct fwnode_endpoint ep;
> +
> + ret = fwnode_graph_parse_endpoint(fwnode, &ep);
> + if (ret) {
> + fwnode_handle_put(fwnode);
> + return ret;
> + }
> +
> + if (ep.port != port)
> + continue;
> + }
> + max_subdevs+
Re: [PATCH v13 05/25] v4l: fwnode: Support generic parsing of graph endpoints in a device
On 09/19/17 12:00, Sakari Ailus wrote:
> Hi Hans,
>
> On Tue, Sep 19, 2017 at 10:40:14AM +0200, Hans Verkuil wrote:
>> On 09/19/2017 10:20 AM, Sakari Ailus wrote:
>>> Hi Hans,
>>>
>>> Thank you for the review.
>>>
>>> On Tue, Sep 19, 2017 at 10:03:27AM +0200, Hans Verkuil wrote:
On 09/15/2017 04:17 PM, Sakari Ailus wrote:
> ...
> +static int __v4l2_async_notifier_parse_fwnode_endpoints(
> + struct device *dev, struct v4l2_async_notifier *notifier,
> + size_t asd_struct_size, unsigned int port, bool has_port,
> + int (*parse_endpoint)(struct device *dev,
> + struct v4l2_fwnode_endpoint *vep,
> + struct v4l2_async_subdev *asd))
> +{
> + struct fwnode_handle *fwnode = NULL;
> + unsigned int max_subdevs = notifier->max_subdevs;
> + int ret;
> +
> + if (WARN_ON(asd_struct_size < sizeof(struct v4l2_async_subdev)))
> + return -EINVAL;
> +
> + for (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(
> + dev_fwnode(dev), fwnode)); ) {
You can replace this by:
while ((fwnode = fwnode_graph_get_next_endpoint(dev_fwnode(dev),
fwnode))) {
> + if (!fwnode_device_is_available(
> + fwnode_graph_get_port_parent(fwnode)))
> + continue;
> +
> + if (has_port) {
> + struct fwnode_endpoint ep;
> +
> + ret = fwnode_graph_parse_endpoint(fwnode, &ep);
> + if (ret) {
> + fwnode_handle_put(fwnode);
> + return ret;
> + }
> +
> + if (ep.port != port)
> + continue;
> + }
> + max_subdevs++;
> + }
> +
> + /* No subdevs to add? Return here. */
> + if (max_subdevs == notifier->max_subdevs)
> + return 0;
> +
> + ret = v4l2_async_notifier_realloc(notifier, max_subdevs);
> + if (ret)
> + return ret;
> +
> + for (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(
> + dev_fwnode(dev), fwnode)); ) {
Same here: this can be a 'while'.
>>>
>>> The fwnode = NULL assignment still needs to be done. A for loop has a
>>> natural initialiser for the loop, I think it's cleaner than using while
>>> here.
>>
>> After the previous while fwnode is NULL again (since that's when the while
>> stops).
>>
>>>
>>> The macro would be implemented this way as well.
>>>
>>> For the loop above this one, I'd use for for consistency: it's the same
>>> loop after all.
>>>
>>> This reminds me --- I'll send the patch for the macro.
>>
>> If this is going to be replaced by a macro, then disregard my comment.
>
> Yes. I just sent that to linux-acpi (as well as devicetree and to you).
>
> ...
>
> +/**
> + * v4l2_async_notifier_parse_fwnode_endpoints - Parse V4L2 fwnode
> endpoints in a
> + * device node
> + * @dev: the device the endpoints of which are to be parsed
> + * @notifier: notifier for @dev
> + * @asd_struct_size: size of the driver's async sub-device struct,
> including
> + *sizeof(struct v4l2_async_subdev). The &struct
> + *v4l2_async_subdev shall be the first member of
> + *the driver's async sub-device struct, i.e. both
> + *begin at the same memory address.
> + * @parse_endpoint: Driver's callback function called on each V4L2 fwnode
> + * endpoint. Optional.
> + * Return: %0 on success
> + * %-ENOTCONN if the endpoint is to be skipped
> but this
> + * should not be considered as an
> error
> + * %-EINVAL if the endpoint configuration is
> invalid
> + *
> + * Parse the fwnode endpoints of the @dev device and populate the async
> sub-
> + * devices array of the notifier. The @parse_endpoint callback function
> is
> + * called for each endpoint with the corresponding async sub-device
> pointer to
> + * let the caller initialize the driver-specific part of the async
> sub-device
> + * structure.
> + *
> + * The notifier memory shall be zeroed before this function is called on
> the
> + * notifier.
> + *
> + * This function may not be called on a registered notifier and may be
> called on
> + * a notifier only once.
> + *
> + * Do not change the notifier's subdevs array, take references to the
> subdevs
> + * array itself or change the notifier's num_subdevs field. This is
> because this
> + * function allocates and reallocates the subdevs array based on pars
Re: [PATCH v13 05/25] v4l: fwnode: Support generic parsing of graph endpoints in a device
Hi Hans,
On Tue, Sep 19, 2017 at 10:40:14AM +0200, Hans Verkuil wrote:
> On 09/19/2017 10:20 AM, Sakari Ailus wrote:
> > Hi Hans,
> >
> > Thank you for the review.
> >
> > On Tue, Sep 19, 2017 at 10:03:27AM +0200, Hans Verkuil wrote:
> >> On 09/15/2017 04:17 PM, Sakari Ailus wrote:
...
> >>> +static int __v4l2_async_notifier_parse_fwnode_endpoints(
> >>> + struct device *dev, struct v4l2_async_notifier *notifier,
> >>> + size_t asd_struct_size, unsigned int port, bool has_port,
> >>> + int (*parse_endpoint)(struct device *dev,
> >>> + struct v4l2_fwnode_endpoint *vep,
> >>> + struct v4l2_async_subdev *asd))
> >>> +{
> >>> + struct fwnode_handle *fwnode = NULL;
> >>> + unsigned int max_subdevs = notifier->max_subdevs;
> >>> + int ret;
> >>> +
> >>> + if (WARN_ON(asd_struct_size < sizeof(struct v4l2_async_subdev)))
> >>> + return -EINVAL;
> >>> +
> >>> + for (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(
> >>> + dev_fwnode(dev), fwnode)); ) {
> >>
> >> You can replace this by:
> >>
> >>while ((fwnode = fwnode_graph_get_next_endpoint(dev_fwnode(dev),
> >> fwnode))) {
> >>
> >>> + if (!fwnode_device_is_available(
> >>> + fwnode_graph_get_port_parent(fwnode)))
> >>> + continue;
> >>> +
> >>> + if (has_port) {
> >>> + struct fwnode_endpoint ep;
> >>> +
> >>> + ret = fwnode_graph_parse_endpoint(fwnode, &ep);
> >>> + if (ret) {
> >>> + fwnode_handle_put(fwnode);
> >>> + return ret;
> >>> + }
> >>> +
> >>> + if (ep.port != port)
> >>> + continue;
> >>> + }
> >>> + max_subdevs++;
> >>> + }
> >>> +
> >>> + /* No subdevs to add? Return here. */
> >>> + if (max_subdevs == notifier->max_subdevs)
> >>> + return 0;
> >>> +
> >>> + ret = v4l2_async_notifier_realloc(notifier, max_subdevs);
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + for (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(
> >>> + dev_fwnode(dev), fwnode)); ) {
> >>
> >> Same here: this can be a 'while'.
> >
> > The fwnode = NULL assignment still needs to be done. A for loop has a
> > natural initialiser for the loop, I think it's cleaner than using while
> > here.
>
> After the previous while fwnode is NULL again (since that's when the while
> stops).
>
> >
> > The macro would be implemented this way as well.
> >
> > For the loop above this one, I'd use for for consistency: it's the same
> > loop after all.
> >
> > This reminds me --- I'll send the patch for the macro.
>
> If this is going to be replaced by a macro, then disregard my comment.
Yes. I just sent that to linux-acpi (as well as devicetree and to you).
...
> >>> +/**
> >>> + * v4l2_async_notifier_parse_fwnode_endpoints - Parse V4L2 fwnode
> >>> endpoints in a
> >>> + * device node
> >>> + * @dev: the device the endpoints of which are to be parsed
> >>> + * @notifier: notifier for @dev
> >>> + * @asd_struct_size: size of the driver's async sub-device struct,
> >>> including
> >>> + *sizeof(struct v4l2_async_subdev). The &struct
> >>> + *v4l2_async_subdev shall be the first member of
> >>> + *the driver's async sub-device struct, i.e. both
> >>> + *begin at the same memory address.
> >>> + * @parse_endpoint: Driver's callback function called on each V4L2 fwnode
> >>> + * endpoint. Optional.
> >>> + * Return: %0 on success
> >>> + * %-ENOTCONN if the endpoint is to be skipped
> >>> but this
> >>> + * should not be considered as an
> >>> error
> >>> + * %-EINVAL if the endpoint configuration is
> >>> invalid
> >>> + *
> >>> + * Parse the fwnode endpoints of the @dev device and populate the async
> >>> sub-
> >>> + * devices array of the notifier. The @parse_endpoint callback function
> >>> is
> >>> + * called for each endpoint with the corresponding async sub-device
> >>> pointer to
> >>> + * let the caller initialize the driver-specific part of the async
> >>> sub-device
> >>> + * structure.
> >>> + *
> >>> + * The notifier memory shall be zeroed before this function is called on
> >>> the
> >>> + * notifier.
> >>> + *
> >>> + * This function may not be called on a registered notifier and may be
> >>> called on
> >>> + * a notifier only once.
> >>> + *
> >>> + * Do not change the notifier's subdevs array, take references to the
> >>> subdevs
> >>> + * array itself or change the notifier's num_subdevs field. This is
> >>> because this
> >>> + * function allocates and reallocates the subdevs array based on parsing
> >>> + * endpoints.
> >>> + *
> >>> + * The @struc
Re: [PATCH v13 05/25] v4l: fwnode: Support generic parsing of graph endpoints in a device
Hi Hans, On Tuesday, 19 September 2017 11:40:14 EEST Hans Verkuil wrote: > On 09/19/2017 10:20 AM, Sakari Ailus wrote: > > On Tue, Sep 19, 2017 at 10:03:27AM +0200, Hans Verkuil wrote: > >> On 09/15/2017 04:17 PM, Sakari Ailus wrote: > >>> Add two functions for parsing devices graph endpoints: > >>> v4l2_async_notifier_parse_fwnode_endpoints and > >>> v4l2_async_notifier_parse_fwnode_endpoints_by_port. The former iterates > >>> over all endpoints whereas the latter only iterates over the endpoints > >>> in a given port. > >>> > >>> The former is mostly useful for existing drivers that currently > >>> implement the iteration over all the endpoints themselves whereas the > >>> latter is especially intended for devices with both sinks and sources: > >>> async sub-devices for external devices connected to the device's sources > >>> will have already been set up, or they are part of the master device. > >>> > >>> Signed-off-by: Sakari Ailus > >>> --- > >>> > >>> drivers/media/v4l2-core/v4l2-async.c | 30 ++ > >>> drivers/media/v4l2-core/v4l2-fwnode.c | 185 +++ > >>> include/media/v4l2-async.h| 24 - > >>> include/media/v4l2-fwnode.h | 117 + > >>> 4 files changed, 354 insertions(+), 2 deletions(-) [snip] > >>> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h > >>> index 68eb22ba571b..83afac48ea6b 100644 > >>> --- a/include/media/v4l2-fwnode.h > >>> +++ b/include/media/v4l2-fwnode.h > >>> @@ -25,6 +25,8 @@ > >>> > >>> #include > >>> > >>> struct fwnode_handle; > >>> +struct v4l2_async_notifier; > >>> +struct v4l2_async_subdev; > >>> > >>> #define V4L2_FWNODE_CSI2_MAX_DATA_LANES 4 > >>> > >>> @@ -201,4 +203,119 @@ int v4l2_fwnode_parse_link(struct fwnode_handle > >>> *fwnode, > >>> */ > >>> void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link); > >>> > >>> +/** > >>> + * v4l2_async_notifier_parse_fwnode_endpoints - Parse V4L2 fwnode > >>> endpoints in a > >>> + * device node > >>> + * @dev: the device the endpoints of which are to be parsed > >>> + * @notifier: notifier for @dev > >>> + * @asd_struct_size: size of the driver's async sub-device struct, > >>> including > >>> + *sizeof(struct v4l2_async_subdev). The &struct > >>> + *v4l2_async_subdev shall be the first member of > >>> + *the driver's async sub-device struct, i.e. both > >>> + *begin at the same memory address. > >>> + * @parse_endpoint: Driver's callback function called on each V4L2 > >>> fwnode > >>> + * endpoint. Optional. > >>> + * Return: %0 on success > >>> + * %-ENOTCONN if the endpoint is to be skipped > >>> but this > >>> + * should not be considered as an > >>> error > >>> + * %-EINVAL if the endpoint configuration is > >>> invalid > >>> + * > >>> + * Parse the fwnode endpoints of the @dev device and populate the async > >>> sub- > >>> + * devices array of the notifier. The @parse_endpoint callback function > >>> is > >>> + * called for each endpoint with the corresponding async sub-device > >>> pointer to > >>> + * let the caller initialize the driver-specific part of the async sub- > >>> device > >>> + * structure. > >>> + * > >>> + * The notifier memory shall be zeroed before this function is called > >>> on the > >>> + * notifier. > >>> + * > >>> + * This function may not be called on a registered notifier and may be > >>> called on > >>> + * a notifier only once. > >>> + * > >>> + * Do not change the notifier's subdevs array, take references to the > >>> subdevs > >>> + * array itself or change the notifier's num_subdevs field. This is > >>> because this > >>> + * function allocates and reallocates the subdevs array based on > >>> parsing > >>> + * endpoints. > >>> + * > >>> + * The @struct v4l2_fwnode_endpoint passed to the callback function > >>> + * @parse_endpoint is released once the function is finished. If there > >>> is a need > >>> + * to retain that configuration, the user needs to allocate memory for > >>> it. > >>> + * > >>> + * Any notifier populated using this function must be released with a > >>> call to > >>> + * v4l2_async_notifier_release() after it has been unregistered and the > >>> async > >>> + * sub-devices are no longer in use, even if the function returned an > >>> error. > >>> + * > >>> + * Return: %0 on success, including when no async sub-devices are found > >>> + * %-ENOMEM if memory allocation failed > >>> + * %-EINVAL if graph or endpoint parsing failed > >>> + * Other error codes as returned by @parse_endpoint > >>> + */ > >>> +int v4l2_async_notifier_parse_fwnode_endpoints( > >>> + struct device *dev, struct v4l2_async_notifier *notifier, > >>> + size_t asd_struct_size, > >>> + int (*parse_endpoint)(stru
Re: [PATCH v13 05/25] v4l: fwnode: Support generic parsing of graph endpoints in a device
On 09/19/2017 10:20 AM, Sakari Ailus wrote:
> Hi Hans,
>
> Thank you for the review.
>
> On Tue, Sep 19, 2017 at 10:03:27AM +0200, Hans Verkuil wrote:
>> On 09/15/2017 04:17 PM, Sakari Ailus wrote:
>>> Add two functions for parsing devices graph endpoints:
>>> v4l2_async_notifier_parse_fwnode_endpoints and
>>> v4l2_async_notifier_parse_fwnode_endpoints_by_port. The former iterates
>>> over all endpoints whereas the latter only iterates over the endpoints in
>>> a given port.
>>>
>>> The former is mostly useful for existing drivers that currently implement
>>> the iteration over all the endpoints themselves whereas the latter is
>>> especially intended for devices with both sinks and sources: async
>>> sub-devices for external devices connected to the device's sources will
>>> have already been set up, or they are part of the master device.
>>>
>>> Signed-off-by: Sakari Ailus
>>> ---
>>> drivers/media/v4l2-core/v4l2-async.c | 30 ++
>>> drivers/media/v4l2-core/v4l2-fwnode.c | 185
>>> ++
>>> include/media/v4l2-async.h| 24 -
>>> include/media/v4l2-fwnode.h | 117 +
>>> 4 files changed, 354 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-async.c
>>> b/drivers/media/v4l2-core/v4l2-async.c
>>> index 831f185ecd47..bf0215dde616 100644
>>> --- a/drivers/media/v4l2-core/v4l2-async.c
>>> +++ b/drivers/media/v4l2-core/v4l2-async.c
>>> @@ -22,6 +22,7 @@
>>>
>>> #include
>>> #include
>>> +#include
>>> #include
>>>
>>> static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev
>>> *asd)
>>> @@ -219,6 +220,35 @@ void v4l2_async_notifier_unregister(struct
>>> v4l2_async_notifier *notifier)
>>> }
>>> EXPORT_SYMBOL(v4l2_async_notifier_unregister);
>>>
>>> +void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier)
>>> +{
>>> + unsigned int i;
>>> +
>>> + if (!notifier->max_subdevs)
>>> + return;
>>> +
>>> + for (i = 0; i < notifier->num_subdevs; i++) {
>>> + struct v4l2_async_subdev *asd = notifier->subdevs[i];
>>> +
>>> + switch (asd->match_type) {
>>> + case V4L2_ASYNC_MATCH_FWNODE:
>>> + fwnode_handle_put(asd->match.fwnode.fwnode);
>>> + break;
>>> + default:
>>> + WARN_ON_ONCE(true);
>>
>> Please add a break here.
>
> Yes.
>
>>
>>> + }
>>> +
>>> + kfree(asd);
>>> + }
>>> +
>>> + notifier->max_subdevs = 0;
>>> + notifier->num_subdevs = 0;
>>> +
>>> + kvfree(notifier->subdevs);
>>> + notifier->subdevs = NULL;
>>> +}
>>> +EXPORT_SYMBOL_GPL(v4l2_async_notifier_release);
>>> +
>>> int v4l2_async_register_subdev(struct v4l2_subdev *sd)
>>> {
>>> struct v4l2_async_notifier *notifier;
>>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
>>> b/drivers/media/v4l2-core/v4l2-fwnode.c
>>> index 706f9e7b90f1..44ee35f6aad5 100644
>>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
>>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
>>> @@ -19,6 +19,7 @@
>>> */
>>> #include
>>> #include
>>> +#include
>>> #include
>>> #include
>>> #include
>>> @@ -26,6 +27,7 @@
>>> #include
>>> #include
>>>
>>> +#include
>>> #include
>>>
>>> enum v4l2_fwnode_bus_type {
>>> @@ -313,6 +315,189 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link
>>> *link)
>>> }
>>> EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);
>>>
>>> +static int v4l2_async_notifier_realloc(struct v4l2_async_notifier
>>> *notifier,
>>> + unsigned int max_subdevs)
>>> +{
>>> + struct v4l2_async_subdev **subdevs;
>>> +
>>> + if (max_subdevs <= notifier->max_subdevs)
>>> + return 0;
>>> +
>>> + subdevs = kvmalloc_array(
>>> + max_subdevs, sizeof(*notifier->subdevs),
>>> + GFP_KERNEL | __GFP_ZERO);
>>> + if (!subdevs)
>>> + return -ENOMEM;
>>> +
>>> + if (notifier->subdevs) {
>>> + memcpy(subdevs, notifier->subdevs,
>>> + sizeof(*subdevs) * notifier->num_subdevs);
>>> +
>>> + kvfree(notifier->subdevs);
>>> + }
>>> +
>>> + notifier->subdevs = subdevs;
>>> + notifier->max_subdevs = max_subdevs;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int v4l2_async_notifier_fwnode_parse_endpoint(
>>> + struct device *dev, struct v4l2_async_notifier *notifier,
>>> + struct fwnode_handle *endpoint, unsigned int asd_struct_size,
>>> + int (*parse_endpoint)(struct device *dev,
>>> + struct v4l2_fwnode_endpoint *vep,
>>> + struct v4l2_async_subdev *asd))
>>> +{
>>> + struct v4l2_async_subdev *asd;
>>> + struct v4l2_fwnode_endpoint *vep;
>>> + int ret = 0;
>>> +
>>> + asd = kzalloc(asd_struct_size, GFP_KERNEL);
>>> + if (!asd)
>>> + return -ENOMEM;
>>> +
>>> + asd->match.fwnode.fwnode =
>>> + fwnode_graph_get_remote_port_parent(endpoint);
>>> + if (!asd->match.fwnode.fwnod
Re: [PATCH v13 05/25] v4l: fwnode: Support generic parsing of graph endpoints in a device
Hi Hans,
Thank you for the review.
On Tue, Sep 19, 2017 at 10:03:27AM +0200, Hans Verkuil wrote:
> On 09/15/2017 04:17 PM, Sakari Ailus wrote:
> > Add two functions for parsing devices graph endpoints:
> > v4l2_async_notifier_parse_fwnode_endpoints and
> > v4l2_async_notifier_parse_fwnode_endpoints_by_port. The former iterates
> > over all endpoints whereas the latter only iterates over the endpoints in
> > a given port.
> >
> > The former is mostly useful for existing drivers that currently implement
> > the iteration over all the endpoints themselves whereas the latter is
> > especially intended for devices with both sinks and sources: async
> > sub-devices for external devices connected to the device's sources will
> > have already been set up, or they are part of the master device.
> >
> > Signed-off-by: Sakari Ailus
> > ---
> > drivers/media/v4l2-core/v4l2-async.c | 30 ++
> > drivers/media/v4l2-core/v4l2-fwnode.c | 185
> > ++
> > include/media/v4l2-async.h| 24 -
> > include/media/v4l2-fwnode.h | 117 +
> > 4 files changed, 354 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c
> > b/drivers/media/v4l2-core/v4l2-async.c
> > index 831f185ecd47..bf0215dde616 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -22,6 +22,7 @@
> >
> > #include
> > #include
> > +#include
> > #include
> >
> > static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev
> > *asd)
> > @@ -219,6 +220,35 @@ void v4l2_async_notifier_unregister(struct
> > v4l2_async_notifier *notifier)
> > }
> > EXPORT_SYMBOL(v4l2_async_notifier_unregister);
> >
> > +void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier)
> > +{
> > + unsigned int i;
> > +
> > + if (!notifier->max_subdevs)
> > + return;
> > +
> > + for (i = 0; i < notifier->num_subdevs; i++) {
> > + struct v4l2_async_subdev *asd = notifier->subdevs[i];
> > +
> > + switch (asd->match_type) {
> > + case V4L2_ASYNC_MATCH_FWNODE:
> > + fwnode_handle_put(asd->match.fwnode.fwnode);
> > + break;
> > + default:
> > + WARN_ON_ONCE(true);
>
> Please add a break here.
Yes.
>
> > + }
> > +
> > + kfree(asd);
> > + }
> > +
> > + notifier->max_subdevs = 0;
> > + notifier->num_subdevs = 0;
> > +
> > + kvfree(notifier->subdevs);
> > + notifier->subdevs = NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(v4l2_async_notifier_release);
> > +
> > int v4l2_async_register_subdev(struct v4l2_subdev *sd)
> > {
> > struct v4l2_async_notifier *notifier;
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> > b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index 706f9e7b90f1..44ee35f6aad5 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -19,6 +19,7 @@
> > */
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -26,6 +27,7 @@
> > #include
> > #include
> >
> > +#include
> > #include
> >
> > enum v4l2_fwnode_bus_type {
> > @@ -313,6 +315,189 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link
> > *link)
> > }
> > EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);
> >
> > +static int v4l2_async_notifier_realloc(struct v4l2_async_notifier
> > *notifier,
> > + unsigned int max_subdevs)
> > +{
> > + struct v4l2_async_subdev **subdevs;
> > +
> > + if (max_subdevs <= notifier->max_subdevs)
> > + return 0;
> > +
> > + subdevs = kvmalloc_array(
> > + max_subdevs, sizeof(*notifier->subdevs),
> > + GFP_KERNEL | __GFP_ZERO);
> > + if (!subdevs)
> > + return -ENOMEM;
> > +
> > + if (notifier->subdevs) {
> > + memcpy(subdevs, notifier->subdevs,
> > + sizeof(*subdevs) * notifier->num_subdevs);
> > +
> > + kvfree(notifier->subdevs);
> > + }
> > +
> > + notifier->subdevs = subdevs;
> > + notifier->max_subdevs = max_subdevs;
> > +
> > + return 0;
> > +}
> > +
> > +static int v4l2_async_notifier_fwnode_parse_endpoint(
> > + struct device *dev, struct v4l2_async_notifier *notifier,
> > + struct fwnode_handle *endpoint, unsigned int asd_struct_size,
> > + int (*parse_endpoint)(struct device *dev,
> > + struct v4l2_fwnode_endpoint *vep,
> > + struct v4l2_async_subdev *asd))
> > +{
> > + struct v4l2_async_subdev *asd;
> > + struct v4l2_fwnode_endpoint *vep;
> > + int ret = 0;
> > +
> > + asd = kzalloc(asd_struct_size, GFP_KERNEL);
> > + if (!asd)
> > + return -ENOMEM;
> > +
> > + asd->match.fwnode.fwnode =
> > + fwnode_graph_get_remote_port_parent(endpoint);
> > + if (!asd->match.fwnode.fwnode) {
> > + dev_warn(dev, "bad remote port parent\
Re: [PATCH v13 05/25] v4l: fwnode: Support generic parsing of graph endpoints in a device
On 09/15/2017 04:17 PM, Sakari Ailus wrote:
> Add two functions for parsing devices graph endpoints:
> v4l2_async_notifier_parse_fwnode_endpoints and
> v4l2_async_notifier_parse_fwnode_endpoints_by_port. The former iterates
> over all endpoints whereas the latter only iterates over the endpoints in
> a given port.
>
> The former is mostly useful for existing drivers that currently implement
> the iteration over all the endpoints themselves whereas the latter is
> especially intended for devices with both sinks and sources: async
> sub-devices for external devices connected to the device's sources will
> have already been set up, or they are part of the master device.
>
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/v4l2-core/v4l2-async.c | 30 ++
> drivers/media/v4l2-core/v4l2-fwnode.c | 185
> ++
> include/media/v4l2-async.h| 24 -
> include/media/v4l2-fwnode.h | 117 +
> 4 files changed, 354 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c
> b/drivers/media/v4l2-core/v4l2-async.c
> index 831f185ecd47..bf0215dde616 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -22,6 +22,7 @@
>
> #include
> #include
> +#include
> #include
>
> static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> @@ -219,6 +220,35 @@ void v4l2_async_notifier_unregister(struct
> v4l2_async_notifier *notifier)
> }
> EXPORT_SYMBOL(v4l2_async_notifier_unregister);
>
> +void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier)
> +{
> + unsigned int i;
> +
> + if (!notifier->max_subdevs)
> + return;
> +
> + for (i = 0; i < notifier->num_subdevs; i++) {
> + struct v4l2_async_subdev *asd = notifier->subdevs[i];
> +
> + switch (asd->match_type) {
> + case V4L2_ASYNC_MATCH_FWNODE:
> + fwnode_handle_put(asd->match.fwnode.fwnode);
> + break;
> + default:
> + WARN_ON_ONCE(true);
Please add a break here.
> + }
> +
> + kfree(asd);
> + }
> +
> + notifier->max_subdevs = 0;
> + notifier->num_subdevs = 0;
> +
> + kvfree(notifier->subdevs);
> + notifier->subdevs = NULL;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_async_notifier_release);
> +
> int v4l2_async_register_subdev(struct v4l2_subdev *sd)
> {
> struct v4l2_async_notifier *notifier;
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 706f9e7b90f1..44ee35f6aad5 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -19,6 +19,7 @@
> */
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -26,6 +27,7 @@
> #include
> #include
>
> +#include
> #include
>
> enum v4l2_fwnode_bus_type {
> @@ -313,6 +315,189 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link)
> }
> EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);
>
> +static int v4l2_async_notifier_realloc(struct v4l2_async_notifier *notifier,
> +unsigned int max_subdevs)
> +{
> + struct v4l2_async_subdev **subdevs;
> +
> + if (max_subdevs <= notifier->max_subdevs)
> + return 0;
> +
> + subdevs = kvmalloc_array(
> + max_subdevs, sizeof(*notifier->subdevs),
> + GFP_KERNEL | __GFP_ZERO);
> + if (!subdevs)
> + return -ENOMEM;
> +
> + if (notifier->subdevs) {
> + memcpy(subdevs, notifier->subdevs,
> +sizeof(*subdevs) * notifier->num_subdevs);
> +
> + kvfree(notifier->subdevs);
> + }
> +
> + notifier->subdevs = subdevs;
> + notifier->max_subdevs = max_subdevs;
> +
> + return 0;
> +}
> +
> +static int v4l2_async_notifier_fwnode_parse_endpoint(
> + struct device *dev, struct v4l2_async_notifier *notifier,
> + struct fwnode_handle *endpoint, unsigned int asd_struct_size,
> + int (*parse_endpoint)(struct device *dev,
> + struct v4l2_fwnode_endpoint *vep,
> + struct v4l2_async_subdev *asd))
> +{
> + struct v4l2_async_subdev *asd;
> + struct v4l2_fwnode_endpoint *vep;
> + int ret = 0;
> +
> + asd = kzalloc(asd_struct_size, GFP_KERNEL);
> + if (!asd)
> + return -ENOMEM;
> +
> + asd->match.fwnode.fwnode =
> + fwnode_graph_get_remote_port_parent(endpoint);
> + if (!asd->match.fwnode.fwnode) {
> + dev_warn(dev, "bad remote port parent\n");
> + ret = -EINVAL;
> + goto out_err;
> + }
> +
> + /* Ignore endpoints the parsing of which failed. */
> + vep = v4l2_fwnode_endpoint_alloc_parse(endpoint);
> + if (IS_ERR(vep)) {
> + ret = PTR_ERR(vep);
> + dev_warn(dev, "unable to parse V4L
[PATCH v13 05/25] v4l: fwnode: Support generic parsing of graph endpoints in a device
Add two functions for parsing devices graph endpoints:
v4l2_async_notifier_parse_fwnode_endpoints and
v4l2_async_notifier_parse_fwnode_endpoints_by_port. The former iterates
over all endpoints whereas the latter only iterates over the endpoints in
a given port.
The former is mostly useful for existing drivers that currently implement
the iteration over all the endpoints themselves whereas the latter is
especially intended for devices with both sinks and sources: async
sub-devices for external devices connected to the device's sources will
have already been set up, or they are part of the master device.
Signed-off-by: Sakari Ailus
---
drivers/media/v4l2-core/v4l2-async.c | 30 ++
drivers/media/v4l2-core/v4l2-fwnode.c | 185 ++
include/media/v4l2-async.h| 24 -
include/media/v4l2-fwnode.h | 117 +
4 files changed, 354 insertions(+), 2 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-async.c
b/drivers/media/v4l2-core/v4l2-async.c
index 831f185ecd47..bf0215dde616 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -22,6 +22,7 @@
#include
#include
+#include
#include
static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
@@ -219,6 +220,35 @@ void v4l2_async_notifier_unregister(struct
v4l2_async_notifier *notifier)
}
EXPORT_SYMBOL(v4l2_async_notifier_unregister);
+void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier)
+{
+ unsigned int i;
+
+ if (!notifier->max_subdevs)
+ return;
+
+ for (i = 0; i < notifier->num_subdevs; i++) {
+ struct v4l2_async_subdev *asd = notifier->subdevs[i];
+
+ switch (asd->match_type) {
+ case V4L2_ASYNC_MATCH_FWNODE:
+ fwnode_handle_put(asd->match.fwnode.fwnode);
+ break;
+ default:
+ WARN_ON_ONCE(true);
+ }
+
+ kfree(asd);
+ }
+
+ notifier->max_subdevs = 0;
+ notifier->num_subdevs = 0;
+
+ kvfree(notifier->subdevs);
+ notifier->subdevs = NULL;
+}
+EXPORT_SYMBOL_GPL(v4l2_async_notifier_release);
+
int v4l2_async_register_subdev(struct v4l2_subdev *sd)
{
struct v4l2_async_notifier *notifier;
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
b/drivers/media/v4l2-core/v4l2-fwnode.c
index 706f9e7b90f1..44ee35f6aad5 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -19,6 +19,7 @@
*/
#include
#include
+#include
#include
#include
#include
@@ -26,6 +27,7 @@
#include
#include
+#include
#include
enum v4l2_fwnode_bus_type {
@@ -313,6 +315,189 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link)
}
EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);
+static int v4l2_async_notifier_realloc(struct v4l2_async_notifier *notifier,
+ unsigned int max_subdevs)
+{
+ struct v4l2_async_subdev **subdevs;
+
+ if (max_subdevs <= notifier->max_subdevs)
+ return 0;
+
+ subdevs = kvmalloc_array(
+ max_subdevs, sizeof(*notifier->subdevs),
+ GFP_KERNEL | __GFP_ZERO);
+ if (!subdevs)
+ return -ENOMEM;
+
+ if (notifier->subdevs) {
+ memcpy(subdevs, notifier->subdevs,
+ sizeof(*subdevs) * notifier->num_subdevs);
+
+ kvfree(notifier->subdevs);
+ }
+
+ notifier->subdevs = subdevs;
+ notifier->max_subdevs = max_subdevs;
+
+ return 0;
+}
+
+static int v4l2_async_notifier_fwnode_parse_endpoint(
+ struct device *dev, struct v4l2_async_notifier *notifier,
+ struct fwnode_handle *endpoint, unsigned int asd_struct_size,
+ int (*parse_endpoint)(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd))
+{
+ struct v4l2_async_subdev *asd;
+ struct v4l2_fwnode_endpoint *vep;
+ int ret = 0;
+
+ asd = kzalloc(asd_struct_size, GFP_KERNEL);
+ if (!asd)
+ return -ENOMEM;
+
+ asd->match.fwnode.fwnode =
+ fwnode_graph_get_remote_port_parent(endpoint);
+ if (!asd->match.fwnode.fwnode) {
+ dev_warn(dev, "bad remote port parent\n");
+ ret = -EINVAL;
+ goto out_err;
+ }
+
+ /* Ignore endpoints the parsing of which failed. */
+ vep = v4l2_fwnode_endpoint_alloc_parse(endpoint);
+ if (IS_ERR(vep)) {
+ ret = PTR_ERR(vep);
+ dev_warn(dev, "unable to parse V4L2 fwnode endpoint (%d)\n",
+ret);
+ goto out_err;
+ }
+
+ ret = parse_endpoint ? parse_endpoint(dev, vep, asd) : 0;
+ if (ret == -ENOTCONN)
+ dev_dbg(dev, "ignoring endpoint %u,%u\n", ve
