Hi Javier and Mauro,
On Sun, Aug 30, 2015 at 12:06:29AM -0300, Mauro Carvalho Chehab wrote:
> From: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
>
> The omap3isp driver parses the graph endpoints to know how many subdevices
> needs to be registered async and register notifiers callbacks for to know
> when these are bound and when the async registrations are completed.
>
> Currently the entities pad are linked with the correct ISP input interface
> when the subdevs are bound but it happens before entitities are registered
> with the media device so that won't work now that the entity links list is
> initialized on device registration.
>
> So instead creating the pad links when the subdevice is bound, create them
> on the complete callback once all the subdevices have been bound but only
> try to create for the ones that have a bus configuration set during bound.
>
> Signed-off-by: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx>
>
> diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
> index b8f6f81d2db2..69e7733d36cd 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2321,26 +2321,33 @@ static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
> struct v4l2_subdev *subdev,
> struct v4l2_async_subdev *asd)
> {
> - struct isp_device *isp = container_of(async, struct isp_device,
> - notifier);
> struct isp_async_subdev *isd =
> container_of(asd, struct isp_async_subdev, asd);
> - int ret;
> -
> - ret = isp_link_entity(isp, &subdev->entity, isd->bus.interface);
> - if (ret < 0)
> - return ret;
>
> isd->sd = subdev;
> isd->sd->host_priv = &isd->bus;
>
> - return ret;
> + return 0;
> }
>
> static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
> {
> struct isp_device *isp = container_of(async, struct isp_device,
> notifier);
> + struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
> + struct v4l2_subdev *sd;
> + struct isp_bus_cfg *bus;
> + int ret;
> +
> + list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
> + /* Only try to link entities whose interface was set on bound */
> + if (sd->host_priv) {
> + bus = (struct isp_bus_cfg *)sd->host_priv;
> + ret = isp_link_entity(isp, &sd->entity, bus->interface);
> + if (ret < 0)
> + return ret;
> + }
> + }
>
> return v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
> }
I think you're working around a problem here, not really fixing it.
This change will create the links only after the media device is registered,
which means the user may obtain a partial enumeration of links if the
enumeration is performed too early.
Before this set, the problem also was that the media device was registered
before the async entities were bound, again making it possible to obtain a
partial enumeration of entities.
What I'd suggest instead is that we split media device initialisation and
registration to the system; that way the media device can be prepared
(entities registered and links created) before it becomes visible to the
user space. I can write a patch for that if you like.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
