[PATCH 0/5] [media] Create pads links after entities registration

[PATCH 0/5] [media] Create pads links after entities registration

[Javier Martinez Canillas]

Hello,

This series changes all the MC media drivers that are currently creating
pads links before registering the media entities with the media device.

The patches are similar to the ones posted for the OMAP3 ISP driver [0]
and depends on Mauro's "[PATCH v8 00/55] MC next generation patches" [1].

The patches just moves the entities registration logic before pads links
creation and in the case of the vsp1, split the pads links creationg from
the entities initialization logic as it was made for the OMAP3 ISP driver.

Unfortunately I don't have hardware to test these patches and they were
only build tested. So testing that I didn't introduce any regression will
be highly appreciated.

[0]: https://lkml.org/lkml/2015/8/26/453
[1]: http://www.spinics.net/lists/linux-samsung-soc/msg47089.html

Best regards,
Javier


Javier Martinez Canillas (5):
  [media] staging: omap4iss: separate links creation from entities init
  [media] v4l: vsp1: create pad links after subdev registration
  [media] v4l: vsp1: separate links creation from entities init
  [media] uvcvideo: create pad links after subdev registration
  [media] smiapp: create pad links after subdev registration

 drivers/media/i2c/smiapp/smiapp-core.c       |  20 +++---
 drivers/media/platform/vsp1/vsp1_drv.c       |  30 +++++---
 drivers/media/platform/vsp1/vsp1_rpf.c       |  29 +++++---
 drivers/media/platform/vsp1/vsp1_rwpf.h      |   5 ++
 drivers/media/platform/vsp1/vsp1_wpf.c       |  40 +++++++----
 drivers/media/usb/uvc/uvc_entity.c           |  16 +++--
 drivers/staging/media/omap4iss/iss.c         | 101 ++++++++++++++++++---------
 drivers/staging/media/omap4iss/iss_csi2.c    |  35 +++++++---
 drivers/staging/media/omap4iss/iss_csi2.h    |   1 +
 drivers/staging/media/omap4iss/iss_ipipeif.c |  29 ++++----
 drivers/staging/media/omap4iss/iss_ipipeif.h |   1 +
 drivers/staging/media/omap4iss/iss_resizer.c |  29 ++++----
 drivers/staging/media/omap4iss/iss_resizer.h |   1 +
 13 files changed, 224 insertions(+), 113 deletions(-)

-- 
2.4.3

[PATCH 1/5] [media] staging: omap4iss: separate links creation from entities init

[Javier Martinez Canillas]

The omap4iss driver initializes the entities and creates the pads links
before the entities are registered with the media device. This does not
work now that object IDs are used to create links so the media_device
has to be set.

Split out the pads links creation from the entity initialization so are
made after the entities registration.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---

 drivers/staging/media/omap4iss/iss.c         | 101 ++++++++++++++++++---------
 drivers/staging/media/omap4iss/iss_csi2.c    |  35 +++++++---
 drivers/staging/media/omap4iss/iss_csi2.h    |   1 +
 drivers/staging/media/omap4iss/iss_ipipeif.c |  29 ++++----
 drivers/staging/media/omap4iss/iss_ipipeif.h |   1 +
 drivers/staging/media/omap4iss/iss_resizer.c |  29 ++++----
 drivers/staging/media/omap4iss/iss_resizer.h |   1 +
 7 files changed, 132 insertions(+), 65 deletions(-)

diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c
index 44b88ff3ba83..076ddd412201 100644
--- a/drivers/staging/media/omap4iss/iss.c
+++ b/drivers/staging/media/omap4iss/iss.c
@@ -1272,6 +1272,68 @@ done:
 	return ret;
 }
 
+/*
+ * iss_create_pads_links() - Pads links creation for the subdevices
+ * @iss : Pointer to ISS device
+ *
+ * return negative error code or zero on success
+ */
+static int iss_create_pads_links(struct iss_device *iss)
+{
+	int ret;
+
+	ret = omap4iss_csi2_create_pads_links(iss);
+	if (ret < 0) {
+		dev_err(iss->dev, "CSI2 pads links creation failed\n");
+		return ret;
+	}
+
+	ret = omap4iss_ipipeif_create_pads_links(iss);
+	if (ret < 0) {
+		dev_err(iss->dev, "ISP IPIPEIF pads links creation failed\n");
+		return ret;
+	}
+
+	ret = omap4iss_resizer_create_pads_links(iss);
+	if (ret < 0) {
+		dev_err(iss->dev, "ISP RESIZER pads links creation failed\n");
+		return ret;
+	}
+
+	/* Connect the submodules. */
+	ret = media_create_pad_link(
+			&iss->csi2a.subdev.entity, CSI2_PAD_SOURCE,
+			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0);
+	if (ret < 0)
+		return ret;
+
+	ret = media_create_pad_link(
+			&iss->csi2b.subdev.entity, CSI2_PAD_SOURCE,
+			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0);
+	if (ret < 0)
+		return ret;
+
+	ret = media_create_pad_link(
+			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP,
+			&iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0);
+	if (ret < 0)
+		return ret;
+
+	ret = media_create_pad_link(
+			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP,
+			&iss->ipipe.subdev.entity, IPIPE_PAD_SINK, 0);
+	if (ret < 0)
+		return ret;
+
+	ret = media_create_pad_link(
+			&iss->ipipe.subdev.entity, IPIPE_PAD_SOURCE_VP,
+			&iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+};
+
 static void iss_cleanup_modules(struct iss_device *iss)
 {
 	omap4iss_csi2_cleanup(iss);
@@ -1314,41 +1376,8 @@ static int iss_initialize_modules(struct iss_device *iss)
 		goto error_resizer;
 	}
 
-	/* Connect the submodules. */
-	ret = media_create_pad_link(
-			&iss->csi2a.subdev.entity, CSI2_PAD_SOURCE,
-			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0);
-	if (ret < 0)
-		goto error_link;
-
-	ret = media_create_pad_link(
-			&iss->csi2b.subdev.entity, CSI2_PAD_SOURCE,
-			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0);
-	if (ret < 0)
-		goto error_link;
-
-	ret = media_create_pad_link(
-			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP,
-			&iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0);
-	if (ret < 0)
-		goto error_link;
-
-	ret = media_create_pad_link(
-			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP,
-			&iss->ipipe.subdev.entity, IPIPE_PAD_SINK, 0);
-	if (ret < 0)
-		goto error_link;
-
-	ret = media_create_pad_link(
-			&iss->ipipe.subdev.entity, IPIPE_PAD_SOURCE_VP,
-			&iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0);
-	if (ret < 0)
-		goto error_link;
-
 	return 0;
 
-error_link:
-	omap4iss_resizer_cleanup(iss);
 error_resizer:
 	omap4iss_ipipe_cleanup(iss);
 error_ipipe:
@@ -1461,10 +1490,16 @@ static int iss_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto error_modules;
 
+	ret = iss_create_pads_links(iss);
+	if (ret < 0)
+		goto error_entities;
+
 	omap4iss_put(iss);
 
 	return 0;
 
+error_entities:
+	iss_unregister_entities(iss);
 error_modules:
 	iss_cleanup_modules(iss);
 error_iss:
diff --git a/drivers/staging/media/omap4iss/iss_csi2.c b/drivers/staging/media/omap4iss/iss_csi2.c
index 50a24e8e8129..907e59edcb04 100644
--- a/drivers/staging/media/omap4iss/iss_csi2.c
+++ b/drivers/staging/media/omap4iss/iss_csi2.c
@@ -1295,16 +1295,8 @@ static int csi2_init_entities(struct iss_csi2_device *csi2, const char *subname)
 	if (ret < 0)
 		goto error_video;
 
-	/* Connect the CSI2 subdev to the video node. */
-	ret = media_create_pad_link(&csi2->subdev.entity, CSI2_PAD_SOURCE,
-				       &csi2->video_out.video.entity, 0, 0);
-	if (ret < 0)
-		goto error_link;
-
 	return 0;
 
-error_link:
-	omap4iss_video_cleanup(&csi2->video_out);
 error_video:
 	media_entity_cleanup(&csi2->subdev.entity);
 	return ret;
@@ -1347,6 +1339,33 @@ int omap4iss_csi2_init(struct iss_device *iss)
 }
 
 /*
+ * omap4iss_csi2_create_pads_links() - CSI2 pads links creation
+ * @iss: Pointer to ISS device
+ *
+ * return negative error code or zero on success
+ */
+int omap4iss_csi2_create_pads_links(struct iss_device *iss)
+{
+	struct iss_csi2_device *csi2a = &iss->csi2a;
+	struct iss_csi2_device *csi2b = &iss->csi2b;
+	int ret;
+
+	/* Connect the CSI2a subdev to the video node. */
+	ret = media_create_pad_link(&csi2a->subdev.entity, CSI2_PAD_SOURCE,
+				    &csi2a->video_out.video.entity, 0, 0);
+	if (ret < 0)
+		return ret;
+
+	/* Connect the CSI2b subdev to the video node. */
+	ret = media_create_pad_link(&csi2b->subdev.entity, CSI2_PAD_SOURCE,
+				    &csi2b->video_out.video.entity, 0, 0);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+/*
  * omap4iss_csi2_cleanup - Routine for module driver cleanup
  */
 void omap4iss_csi2_cleanup(struct iss_device *iss)
diff --git a/drivers/staging/media/omap4iss/iss_csi2.h b/drivers/staging/media/omap4iss/iss_csi2.h
index 3b37978a3bdf..ad9a02d2a576 100644
--- a/drivers/staging/media/omap4iss/iss_csi2.h
+++ b/drivers/staging/media/omap4iss/iss_csi2.h
@@ -151,6 +151,7 @@ struct iss_csi2_device {
 void omap4iss_csi2_isr(struct iss_csi2_device *csi2);
 int omap4iss_csi2_reset(struct iss_csi2_device *csi2);
 int omap4iss_csi2_init(struct iss_device *iss);
+int omap4iss_csi2_create_pads_links(struct iss_device *iss);
 void omap4iss_csi2_cleanup(struct iss_device *iss);
 void omap4iss_csi2_unregister_entities(struct iss_csi2_device *csi2);
 int omap4iss_csi2_register_entities(struct iss_csi2_device *csi2,
diff --git a/drivers/staging/media/omap4iss/iss_ipipeif.c b/drivers/staging/media/omap4iss/iss_ipipeif.c
index e46b2c07bd5d..36a570e60496 100644
--- a/drivers/staging/media/omap4iss/iss_ipipeif.c
+++ b/drivers/staging/media/omap4iss/iss_ipipeif.c
@@ -762,18 +762,7 @@ static int ipipeif_init_entities(struct iss_ipipeif_device *ipipeif)
 	ipipeif->video_out.bpl_zero_padding = 1;
 	ipipeif->video_out.bpl_max = 0x1ffe0;
 
-	ret = omap4iss_video_init(&ipipeif->video_out, "ISP IPIPEIF");
-	if (ret < 0)
-		return ret;
-
-	/* Connect the IPIPEIF subdev to the video node. */
-	ret = media_create_pad_link(&ipipeif->subdev.entity,
-				       IPIPEIF_PAD_SOURCE_ISIF_SF,
-				       &ipipeif->video_out.video.entity, 0, 0);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+	return omap4iss_video_init(&ipipeif->video_out, "ISP IPIPEIF");
 }
 
 void omap4iss_ipipeif_unregister_entities(struct iss_ipipeif_device *ipipeif)
@@ -826,6 +815,22 @@ int omap4iss_ipipeif_init(struct iss_device *iss)
 }
 
 /*
+ * omap4iss_ipipeif_create_pads_links() - IPIPEIF pads links creation
+ * @iss: Pointer to ISS device
+ *
+ * return negative error code or zero on success
+ */
+int omap4iss_ipipeif_create_pads_links(struct iss_device *iss)
+{
+	struct iss_ipipeif_device *ipipeif = &iss->ipipeif;
+
+	/* Connect the IPIPEIF subdev to the video node. */
+	return media_create_pad_link(&ipipeif->subdev.entity,
+				     IPIPEIF_PAD_SOURCE_ISIF_SF,
+				     &ipipeif->video_out.video.entity, 0, 0);
+}
+
+/*
  * omap4iss_ipipeif_cleanup - IPIPEIF module cleanup.
  * @iss: Device pointer specific to the OMAP4 ISS.
  */
diff --git a/drivers/staging/media/omap4iss/iss_ipipeif.h b/drivers/staging/media/omap4iss/iss_ipipeif.h
index cbdccb982eee..28357a65ae97 100644
--- a/drivers/staging/media/omap4iss/iss_ipipeif.h
+++ b/drivers/staging/media/omap4iss/iss_ipipeif.h
@@ -78,6 +78,7 @@ struct iss_ipipeif_device {
 struct iss_device;
 
 int omap4iss_ipipeif_init(struct iss_device *iss);
+int omap4iss_ipipeif_create_pads_links(struct iss_device *iss);
 void omap4iss_ipipeif_cleanup(struct iss_device *iss);
 int omap4iss_ipipeif_register_entities(struct iss_ipipeif_device *ipipeif,
 	struct v4l2_device *vdev);
diff --git a/drivers/staging/media/omap4iss/iss_resizer.c b/drivers/staging/media/omap4iss/iss_resizer.c
index bc5001002cc5..5de093715f98 100644
--- a/drivers/staging/media/omap4iss/iss_resizer.c
+++ b/drivers/staging/media/omap4iss/iss_resizer.c
@@ -806,18 +806,7 @@ static int resizer_init_entities(struct iss_resizer_device *resizer)
 	resizer->video_out.bpl_zero_padding = 1;
 	resizer->video_out.bpl_max = 0x1ffe0;
 
-	ret = omap4iss_video_init(&resizer->video_out, "ISP resizer a");
-	if (ret < 0)
-		return ret;
-
-	/* Connect the RESIZER subdev to the video node. */
-	ret = media_create_pad_link(&resizer->subdev.entity,
-				       RESIZER_PAD_SOURCE_MEM,
-				       &resizer->video_out.video.entity, 0, 0);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+	return omap4iss_video_init(&resizer->video_out, "ISP resizer a");
 }
 
 void omap4iss_resizer_unregister_entities(struct iss_resizer_device *resizer)
@@ -870,6 +859,22 @@ int omap4iss_resizer_init(struct iss_device *iss)
 }
 
 /*
+ * omap4iss_resizer_create_pads_links() - RESIZER pads links creation
+ * @iss: Pointer to ISS device
+ *
+ * return negative error code or zero on success
+ */
+int omap4iss_resizer_create_pads_links(struct iss_device *iss)
+{
+	struct iss_resizer_device *resizer = &iss->resizer;
+
+	/* Connect the RESIZER subdev to the video node. */
+	return media_create_pad_link(&resizer->subdev.entity,
+				     RESIZER_PAD_SOURCE_MEM,
+				     &resizer->video_out.video.entity, 0, 0);
+}
+
+/*
  * omap4iss_resizer_cleanup - RESIZER module cleanup.
  * @iss: Device pointer specific to the OMAP4 ISS.
  */
diff --git a/drivers/staging/media/omap4iss/iss_resizer.h b/drivers/staging/media/omap4iss/iss_resizer.h
index 3727498b06a3..00ecc151e511 100644
--- a/drivers/staging/media/omap4iss/iss_resizer.h
+++ b/drivers/staging/media/omap4iss/iss_resizer.h
@@ -61,6 +61,7 @@ struct iss_resizer_device {
 struct iss_device;
 
 int omap4iss_resizer_init(struct iss_device *iss);
+int omap4iss_resizer_create_pads_links(struct iss_device *iss);
 void omap4iss_resizer_cleanup(struct iss_device *iss);
 int omap4iss_resizer_register_entities(struct iss_resizer_device *resizer,
 	struct v4l2_device *vdev);
-- 
2.4.3

[PATCH 2/5] [media] v4l: vsp1: create pad links after subdev registration

[Javier Martinez Canillas]

The vsp1 driver creates the pads links before the media entities are
registered with the media device. This doesn't work now that object
IDs are used to create links so the media_device has to be set.

Move entities registration logic before pads links creation.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---

 drivers/media/platform/vsp1/vsp1_drv.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
index 9cd94a76a9ed..2aa427d3ff39 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -250,6 +250,14 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
 		list_add_tail(&wpf->entity.list_dev, &vsp1->entities);
 	}
 
+	/* Register all subdevs. */
+	list_for_each_entry(entity, &vsp1->entities, list_dev) {
+		ret = v4l2_device_register_subdev(&vsp1->v4l2_dev,
+						  &entity->subdev);
+		if (ret < 0)
+			goto done;
+	}
+
 	/* Create links. */
 	list_for_each_entry(entity, &vsp1->entities, list_dev) {
 		if (entity->type == VSP1_ENTITY_LIF ||
@@ -269,14 +277,6 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
 			return ret;
 	}
 
-	/* Register all subdevs. */
-	list_for_each_entry(entity, &vsp1->entities, list_dev) {
-		ret = v4l2_device_register_subdev(&vsp1->v4l2_dev,
-						  &entity->subdev);
-		if (ret < 0)
-			goto done;
-	}
-
 	ret = v4l2_device_register_subdev_nodes(&vsp1->v4l2_dev);
 
 done:
-- 
2.4.3

[PATCH 3/5] [media] v4l: vsp1: separate links creation from entities init

[Javier Martinez Canillas]

The vsp1 driver initializes the entities and creates the pads links
before the entities are registered with the media device. This doesn't
work now that object IDs are used to create links so the media_device
has to be set.

Split out the pads links creation from the entity initialization so are
made after the entities registration.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---

 drivers/media/platform/vsp1/vsp1_drv.c  | 14 ++++++++++--
 drivers/media/platform/vsp1/vsp1_rpf.c  | 29 ++++++++++++++++--------
 drivers/media/platform/vsp1/vsp1_rwpf.h |  5 +++++
 drivers/media/platform/vsp1/vsp1_wpf.c  | 40 ++++++++++++++++++++-------------
 4 files changed, 62 insertions(+), 26 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
index 2aa427d3ff39..8f995d267646 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -260,9 +260,19 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
 
 	/* Create links. */
 	list_for_each_entry(entity, &vsp1->entities, list_dev) {
-		if (entity->type == VSP1_ENTITY_LIF ||
-		    entity->type == VSP1_ENTITY_RPF)
+		if (entity->type == VSP1_ENTITY_LIF) {
+			ret = vsp1_wpf_create_pads_links(vsp1, entity);
+			if (ret < 0)
+				goto done;
+			continue;
+		}
+
+		if (entity->type == VSP1_ENTITY_RPF) {
+			ret = vsp1_rpf_create_pads_links(vsp1, entity);
+			if (ret < 0)
+				goto done;
 			continue;
+		}
 
 		ret = vsp1_create_links(vsp1, entity);
 		if (ret < 0)
diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c
index b60a528a8fe8..38aebdf691b5 100644
--- a/drivers/media/platform/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rpf.c
@@ -277,18 +277,29 @@ struct vsp1_rwpf *vsp1_rpf_create(struct vsp1_device *vsp1, unsigned int index)
 
 	rpf->entity.video = video;
 
-	/* Connect the video device to the RPF. */
-	ret = media_create_pad_link(&rpf->video.video.entity, 0,
-				       &rpf->entity.subdev.entity,
-				       RWPF_PAD_SINK,
-				       MEDIA_LNK_FL_ENABLED |
-				       MEDIA_LNK_FL_IMMUTABLE);
-	if (ret < 0)
-		goto error;
-
 	return rpf;
 
 error:
 	vsp1_entity_destroy(&rpf->entity);
 	return ERR_PTR(ret);
 }
+
+/*
+ * vsp1_rpf_create_pads_links_create_pads_links() - RPF pads links creation
+ * @vsp1: Pointer to VSP1 device
+ * @entity: Pointer to VSP1 entity
+ *
+ * return negative error code or zero on success
+ */
+int vsp1_rpf_create_pads_links(struct vsp1_device *vsp1,
+			       struct vsp1_entity *entity)
+{
+	struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
+
+	/* Connect the video device to the RPF. */
+	return media_create_pad_link(&rpf->video.video.entity, 0,
+				     &rpf->entity.subdev.entity,
+				     RWPF_PAD_SINK,
+				     MEDIA_LNK_FL_ENABLED |
+				     MEDIA_LNK_FL_IMMUTABLE);
+}
diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.h b/drivers/media/platform/vsp1/vsp1_rwpf.h
index f452dce1a931..6638b3587369 100644
--- a/drivers/media/platform/vsp1/vsp1_rwpf.h
+++ b/drivers/media/platform/vsp1/vsp1_rwpf.h
@@ -50,6 +50,11 @@ static inline struct vsp1_rwpf *to_rwpf(struct v4l2_subdev *subdev)
 struct vsp1_rwpf *vsp1_rpf_create(struct vsp1_device *vsp1, unsigned int index);
 struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device *vsp1, unsigned int index);
 
+int vsp1_rpf_create_pads_links(struct vsp1_device *vsp1,
+			       struct vsp1_entity *entity);
+int vsp1_wpf_create_pads_links(struct vsp1_device *vsp1,
+			       struct vsp1_entity *entity);
+
 int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev,
 			     struct v4l2_subdev_pad_config *cfg,
 			     struct v4l2_subdev_mbus_code_enum *code);
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
index d39aa4b8aea1..1be363e4f741 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -220,7 +220,6 @@ struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device *vsp1, unsigned int index)
 	struct v4l2_subdev *subdev;
 	struct vsp1_video *video;
 	struct vsp1_rwpf *wpf;
-	unsigned int flags;
 	int ret;
 
 	wpf = devm_kzalloc(vsp1->dev, sizeof(*wpf), GFP_KERNEL);
@@ -276,20 +275,6 @@ struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device *vsp1, unsigned int index)
 		goto error;
 
 	wpf->entity.video = video;
-
-	/* Connect the video device to the WPF. All connections are immutable
-	 * except for the WPF0 source link if a LIF is present.
-	 */
-	flags = MEDIA_LNK_FL_ENABLED;
-	if (!(vsp1->pdata.features & VSP1_HAS_LIF) || index != 0)
-		flags |= MEDIA_LNK_FL_IMMUTABLE;
-
-	ret = media_create_pad_link(&wpf->entity.subdev.entity,
-				       RWPF_PAD_SOURCE,
-				       &wpf->video.video.entity, 0, flags);
-	if (ret < 0)
-		goto error;
-
 	wpf->entity.sink = &wpf->video.video.entity;
 
 	return wpf;
@@ -298,3 +283,28 @@ error:
 	vsp1_entity_destroy(&wpf->entity);
 	return ERR_PTR(ret);
 }
+
+/*
+ * vsp1_wpf_create_pads_links_create_pads_links() - RPF pads links creation
+ * @vsp1: Pointer to VSP1 device
+ * @entity: Pointer to VSP1 entity
+ *
+ * return negative error code or zero on success
+ */
+int vsp1_wpf_create_pads_links(struct vsp1_device *vsp1,
+			       struct vsp1_entity *entity)
+{
+	struct vsp1_rwpf *wpf = to_rwpf(&entity->subdev);
+	unsigned int flags;
+
+	/* Connect the video device to the WPF. All connections are immutable
+	 * except for the WPF0 source link if a LIF is present.
+	 */
+	flags = MEDIA_LNK_FL_ENABLED;
+	if (!(vsp1->pdata.features & VSP1_HAS_LIF) || entity->index != 0)
+		flags |= MEDIA_LNK_FL_IMMUTABLE;
+
+	return media_create_pad_link(&wpf->entity.subdev.entity,
+				     RWPF_PAD_SOURCE,
+				     &wpf->video.video.entity, 0, flags);
+}
-- 
2.4.3

[PATCH 4/5] [media] uvcvideo: create pad links after subdev registration

[Javier Martinez Canillas]

The uvc driver creates the pads links before the media entity is
registered with the media device. This doesn't work now that obj
IDs are used to create links so the media_device has to be set.

Move entities registration logic before pads links creation.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---

 drivers/media/usb/uvc/uvc_entity.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_entity.c b/drivers/media/usb/uvc/uvc_entity.c
index 429e428ccd93..9dde1f86cc4b 100644
--- a/drivers/media/usb/uvc/uvc_entity.c
+++ b/drivers/media/usb/uvc/uvc_entity.c
@@ -37,6 +37,10 @@ static int uvc_mc_register_entity(struct uvc_video_chain *chain,
 	if (sink == NULL)
 		return 0;
 
+	ret = v4l2_device_register_subdev(&chain->dev->vdev, &entity->subdev);
+	if (ret < 0)
+		return ret;
+
 	for (i = 0; i < entity->num_pads; ++i) {
 		struct media_entity *source;
 		struct uvc_entity *remote;
@@ -46,8 +50,10 @@ static int uvc_mc_register_entity(struct uvc_video_chain *chain,
 			continue;
 
 		remote = uvc_entity_by_id(chain->dev, entity->baSourceID[i]);
-		if (remote == NULL)
-			return -EINVAL;
+		if (remote == NULL) {
+			ret = -EINVAL;
+			goto error;
+		}
 
 		source = (UVC_ENTITY_TYPE(remote) == UVC_TT_STREAMING)
 		       ? (remote->vdev ? &remote->vdev->entity : NULL)
@@ -59,13 +65,15 @@ static int uvc_mc_register_entity(struct uvc_video_chain *chain,
 		ret = media_create_pad_link(source, remote_pad,
 					       sink, i, flags);
 		if (ret < 0)
-			return ret;
+			goto error;
 	}
 
 	if (UVC_ENTITY_TYPE(entity) == UVC_TT_STREAMING)
 		return 0;
 
-	return v4l2_device_register_subdev(&chain->dev->vdev, &entity->subdev);
+error:
+	v4l2_device_unregister_subdev(&entity->subdev);
+	return ret;
 }
 
 static struct v4l2_subdev_ops uvc_subdev_ops = {
-- 
2.4.3

[PATCH 5/5] [media] smiapp: create pad links after subdev registration

[Javier Martinez Canillas]

The smiapp driver creates the pads links before the media entity is
registered with the media device. This doesn't work now that object
IDs are used to create links so the media_device has to be set.

Move entity registration logic before pads links creation.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

---

 drivers/media/i2c/smiapp/smiapp-core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 5aa49eb393a9..938201789ebc 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2495,23 +2495,23 @@ static int smiapp_register_subdevs(struct smiapp_sensor *sensor)
 			return rval;
 		}
 
-		rval = media_create_pad_link(&this->sd.entity,
-						this->source_pad,
-						&last->sd.entity,
-						last->sink_pad,
-						MEDIA_LNK_FL_ENABLED |
-						MEDIA_LNK_FL_IMMUTABLE);
+		rval = v4l2_device_register_subdev(sensor->src->sd.v4l2_dev,
+						   &this->sd);
 		if (rval) {
 			dev_err(&client->dev,
-				"media_create_pad_link failed\n");
+				"v4l2_device_register_subdev failed\n");
 			return rval;
 		}
 
-		rval = v4l2_device_register_subdev(sensor->src->sd.v4l2_dev,
-						   &this->sd);
+		rval = media_create_pad_link(&this->sd.entity,
+					     this->source_pad,
+					     &last->sd.entity,
+					     last->sink_pad,
+					     MEDIA_LNK_FL_ENABLED |
+					     MEDIA_LNK_FL_IMMUTABLE);
 		if (rval) {
 			dev_err(&client->dev,
-				"v4l2_device_register_subdev failed\n");
+				"media_create_pad_link failed\n");
 			return rval;
 		}
 	}
-- 
2.4.3

Re: [PATCH 4/5] [media] uvcvideo: create pad links after subdev registration

[Javier Martinez Canillas]

Hello,

On 09/03/2015 06:00 PM, Javier Martinez Canillas wrote:
> The uvc driver creates the pads links before the media entity is
> registered with the media device. This doesn't work now that obj
> IDs are used to create links so the media_device has to be set.
> 
> Move entities registration logic before pads links creation.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
> 
>  drivers/media/usb/uvc/uvc_entity.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_entity.c b/drivers/media/usb/uvc/uvc_entity.c
> index 429e428ccd93..9dde1f86cc4b 100644
> --- a/drivers/media/usb/uvc/uvc_entity.c
> +++ b/drivers/media/usb/uvc/uvc_entity.c
> @@ -37,6 +37,10 @@ static int uvc_mc_register_entity(struct uvc_video_chain *chain,
>  	if (sink == NULL)
>  		return 0;
>  
> +	ret = v4l2_device_register_subdev(&chain->dev->vdev, &entity->subdev);

Testing this patch on an Exynos Chromebook that has a built-in USB camera, I
noticed that v4l2_device_register_subdev() was wrongly called for UVC entities
with UVC_ENTITY_TYPE() == UVC_TT_STREAMING.

So I have the following [0] v2 patch. This patch was tested on an Exynos5800
Peach Pi Chromebook using qv4l2 to test video capture and the output of the
media-ctl -p command was compared with and without the MC next gen patches
to verify that was identical in both cases.

The media-ctl -p output is: http://hastebin.com/enalitofoz.coffee

And the mc_next_gen -e -i -I -l output is http://hastebin.com/umevuragaq.pas

Normally I would just resend the complete series but since there are so many
in-flight patches, I preferred to only re-send this patch one in this thread.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America


[0]:
>From 8be356e77eeefdc5c0738dd429205f3398c5b76c Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <javier@osg.samsung.com>
Date: Thu, 3 Sep 2015 13:46:06 +0200
Subject: [PATCH v2 4/5] [media] uvcvideo: create pad links after subdev
 registration

The uvc driver creates the pads links before the media entity is
registered with the media device. This doesn't work now that obj
IDs are used to create links so the media_device has to be set.

Move entities registration logic before pads links creation.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---

Changes since v1:
 - Don't try to register a UVC entity subdev for type UVC_TT_STREAMING.

 drivers/media/usb/uvc/uvc_entity.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_entity.c b/drivers/media/usb/uvc/uvc_entity.c
index 429e428ccd93..7f82b65b238e 100644
--- a/drivers/media/usb/uvc/uvc_entity.c
+++ b/drivers/media/usb/uvc/uvc_entity.c
@@ -26,6 +26,15 @@
 static int uvc_mc_register_entity(struct uvc_video_chain *chain,
 	struct uvc_entity *entity)
 {
+	if (UVC_ENTITY_TYPE(entity) == UVC_TT_STREAMING)
+		return 0;
+
+	return v4l2_device_register_subdev(&chain->dev->vdev, &entity->subdev);
+}
+
+static int uvc_mc_create_pads_links(struct uvc_video_chain *chain,
+				    struct uvc_entity *entity)
+{
 	const u32 flags = MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE;
 	struct media_entity *sink;
 	unsigned int i;
@@ -62,10 +71,7 @@ static int uvc_mc_register_entity(struct uvc_video_chain *chain,
 			return ret;
 	}
 
-	if (UVC_ENTITY_TYPE(entity) == UVC_TT_STREAMING)
-		return 0;
-
-	return v4l2_device_register_subdev(&chain->dev->vdev, &entity->subdev);
+	return 0;
 }
 
 static struct v4l2_subdev_ops uvc_subdev_ops = {
@@ -124,5 +130,14 @@ int uvc_mc_register_entities(struct uvc_video_chain *chain)
 		}
 	}
 
+	list_for_each_entry(entity, &chain->entities, chain) {
+		ret = uvc_mc_create_pads_links(chain, entity);
+		if (ret < 0) {
+			uvc_printk(KERN_INFO, "Failed to create pads links for "
+				   "entity %u\n", entity->id);
+			return ret;
+		}
+	}
+
 	return 0;
 }
-- 
2.4.3

Re: [PATCH 5/5] [media] smiapp: create pad links after subdev registration

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Thursday 03 September 2015 18:00:36 Javier Martinez Canillas wrote:
> The smiapp driver creates the pads links before the media entity is
> registered with the media device. This doesn't work now that object
> IDs are used to create links so the media_device has to be set.
> 
> Move entity registration logic before pads links creation.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> 
>  drivers/media/i2c/smiapp/smiapp-core.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c
> b/drivers/media/i2c/smiapp/smiapp-core.c index 5aa49eb393a9..938201789ebc
> 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -2495,23 +2495,23 @@ static int smiapp_register_subdevs(struct
> smiapp_sensor *sensor) return rval;
>  		}
> 
> -		rval = media_create_pad_link(&this->sd.entity,
> -						this->source_pad,
> -						&last->sd.entity,
> -						last->sink_pad,
> -						MEDIA_LNK_FL_ENABLED |
> -						MEDIA_LNK_FL_IMMUTABLE);
> +		rval = v4l2_device_register_subdev(sensor->src->sd.v4l2_dev,
> +						   &this->sd);
>  		if (rval) {
>  			dev_err(&client->dev,
> -				"media_create_pad_link failed\n");
> +				"v4l2_device_register_subdev failed\n");
>  			return rval;
>  		}
> 
> -		rval = v4l2_device_register_subdev(sensor->src->sd.v4l2_dev,
> -						   &this->sd);
> +		rval = media_create_pad_link(&this->sd.entity,
> +					     this->source_pad,
> +					     &last->sd.entity,
> +					     last->sink_pad,
> +					     MEDIA_LNK_FL_ENABLED |
> +					     MEDIA_LNK_FL_IMMUTABLE);
>  		if (rval) {
>  			dev_err(&client->dev,
> -				"v4l2_device_register_subdev failed\n");
> +				"media_create_pad_link failed\n");
>  			return rval;
>  		}
>  	}

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/5] [media] uvcvideo: create pad links after subdev registration

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Monday 07 September 2015 16:10:25 Javier Martinez Canillas wrote:

[snip]

> From 8be356e77eeefdc5c0738dd429205f3398c5b76c Mon Sep 17 00:00:00 2001
> From: Javier Martinez Canillas <javier@osg.samsung.com>
> Date: Thu, 3 Sep 2015 13:46:06 +0200
> Subject: [PATCH v2 4/5] [media] uvcvideo: create pad links after subdev
>  registration
> 
> The uvc driver creates the pads links before the media entity is
> registered with the media device. This doesn't work now that obj
> IDs are used to create links so the media_device has to be set.
> 
> Move entities registration logic before pads links creation.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
> 
> Changes since v1:
>  - Don't try to register a UVC entity subdev for type UVC_TT_STREAMING.
> 
>  drivers/media/usb/uvc/uvc_entity.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_entity.c
> b/drivers/media/usb/uvc/uvc_entity.c index 429e428ccd93..7f82b65b238e
> 100644
> --- a/drivers/media/usb/uvc/uvc_entity.c
> +++ b/drivers/media/usb/uvc/uvc_entity.c
> @@ -26,6 +26,15 @@
>  static int uvc_mc_register_entity(struct uvc_video_chain *chain,
>         struct uvc_entity *entity)
>  {
> +       if (UVC_ENTITY_TYPE(entity) == UVC_TT_STREAMING)
> +               return 0;
> +
> +       return v4l2_device_register_subdev(&chain->dev->vdev,
> &entity->subdev);
> +}
> +
> +static int uvc_mc_create_pads_links(struct uvc_video_chain *chain,
> +                                   struct uvc_entity *entity)
> +{
>         const u32 flags = MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE;
>         struct media_entity *sink;
>         unsigned int i;
> @@ -62,10 +71,7 @@ static int uvc_mc_register_entity(struct uvc_video_chain
> *chain, return ret;
>         }
>  
> -       if (UVC_ENTITY_TYPE(entity) == UVC_TT_STREAMING)
> -               return 0;
> -
> -       return v4l2_device_register_subdev(&chain->dev->vdev,
> &entity->subdev);
> +       return 0;
>  }
>  
>  static struct v4l2_subdev_ops uvc_subdev_ops = {
> @@ -124,5 +130,14 @@ int uvc_mc_register_entities(struct uvc_video_chain
> *chain) }
>         }
>  
> +       list_for_each_entry(entity, &chain->entities, chain) {
> +               ret = uvc_mc_create_pads_links(chain, entity);

You can call this uvc_mc_create_links(), there's no other type of links in the 
driver.

> +               if (ret < 0) {
> +                       uvc_printk(KERN_INFO, "Failed to create pads links
> for "

Same here, I'd s/pad links/links/.

> +                                  "entity %u\n", entity->id);
> +                       return ret;
> +               }
> +       }

This creates three loops, and I think that's one too much. The reason why init 
and register are separate is that the latter creates links, which requires all 
entities to be initialized. If you move link create after registration I 
believe you can init and register in a single loop (just move the 
v4l2_device_register_subdev() call in the appropriate location in 
uvc_mc_init_entity()) and then create links in a second loop.

>         return 0;
>  }

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/5] [media] v4l: vsp1: create pad links after subdev registration

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Thursday 03 September 2015 18:00:33 Javier Martinez Canillas wrote:
> The vsp1 driver creates the pads links before the media entities are
> registered with the media device. This doesn't work now that object
> IDs are used to create links so the media_device has to be set.
> 
> Move entities registration logic before pads links creation.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> 
>  drivers/media/platform/vsp1/vsp1_drv.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c
> b/drivers/media/platform/vsp1/vsp1_drv.c index 9cd94a76a9ed..2aa427d3ff39
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -250,6 +250,14 @@ static int vsp1_create_entities(struct vsp1_device
> *vsp1) list_add_tail(&wpf->entity.list_dev, &vsp1->entities);
>  	}
> 
> +	/* Register all subdevs. */
> +	list_for_each_entry(entity, &vsp1->entities, list_dev) {
> +		ret = v4l2_device_register_subdev(&vsp1->v4l2_dev,
> +						  &entity->subdev);
> +		if (ret < 0)
> +			goto done;
> +	}
> +
>  	/* Create links. */
>  	list_for_each_entry(entity, &vsp1->entities, list_dev) {
>  		if (entity->type == VSP1_ENTITY_LIF ||
> @@ -269,14 +277,6 @@ static int vsp1_create_entities(struct vsp1_device
> *vsp1) return ret;
>  	}
> 
> -	/* Register all subdevs. */
> -	list_for_each_entry(entity, &vsp1->entities, list_dev) {
> -		ret = v4l2_device_register_subdev(&vsp1->v4l2_dev,
> -						  &entity->subdev);
> -		if (ret < 0)
> -			goto done;
> -	}
> -
>  	ret = v4l2_device_register_subdev_nodes(&vsp1->v4l2_dev);
> 
>  done:

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/5] [media] v4l: vsp1: separate links creation from entities init

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Thursday 03 September 2015 18:00:34 Javier Martinez Canillas wrote:
> The vsp1 driver initializes the entities and creates the pads links
> before the entities are registered with the media device. This doesn't
> work now that object IDs are used to create links so the media_device
> has to be set.
> 
> Split out the pads links creation from the entity initialization so are
> made after the entities registration.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
> 
>  drivers/media/platform/vsp1/vsp1_drv.c  | 14 ++++++++++--
>  drivers/media/platform/vsp1/vsp1_rpf.c  | 29 ++++++++++++++++--------
>  drivers/media/platform/vsp1/vsp1_rwpf.h |  5 +++++
>  drivers/media/platform/vsp1/vsp1_wpf.c  | 40 +++++++++++++++++-------------
>  4 files changed, 62 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c
> b/drivers/media/platform/vsp1/vsp1_drv.c index 2aa427d3ff39..8f995d267646
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -260,9 +260,19 @@ static int vsp1_create_entities(struct vsp1_device
> *vsp1)
> 
>  	/* Create links. */
>  	list_for_each_entry(entity, &vsp1->entities, list_dev) {
> -		if (entity->type == VSP1_ENTITY_LIF ||
> -		    entity->type == VSP1_ENTITY_RPF)
> +		if (entity->type == VSP1_ENTITY_LIF) {
> +			ret = vsp1_wpf_create_pads_links(vsp1, entity);

Could you please s/pads_links/links/ ? There's no other type of links handled 
by the driver.

> +			if (ret < 0)
> +				goto done;
> +			continue;

I would use

	} else if (...) {

instead of a continue.

> +		}
> +
> +		if (entity->type == VSP1_ENTITY_RPF) {
> +			ret = vsp1_rpf_create_pads_links(vsp1, entity);
> +			if (ret < 0)
> +				goto done;
>  			continue;

Same here.

Apart from that,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +		}
> 
>  		ret = vsp1_create_links(vsp1, entity);
>  		if (ret < 0)
> diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c
> b/drivers/media/platform/vsp1/vsp1_rpf.c index b60a528a8fe8..38aebdf691b5
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_rpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rpf.c
> @@ -277,18 +277,29 @@ struct vsp1_rwpf *vsp1_rpf_create(struct vsp1_device
> *vsp1, unsigned int index)
> 
>  	rpf->entity.video = video;
> 
> -	/* Connect the video device to the RPF. */
> -	ret = media_create_pad_link(&rpf->video.video.entity, 0,
> -				       &rpf->entity.subdev.entity,
> -				       RWPF_PAD_SINK,
> -				       MEDIA_LNK_FL_ENABLED |
> -				       MEDIA_LNK_FL_IMMUTABLE);
> -	if (ret < 0)
> -		goto error;
> -
>  	return rpf;
> 
>  error:
>  	vsp1_entity_destroy(&rpf->entity);
>  	return ERR_PTR(ret);
>  }
> +
> +/*
> + * vsp1_rpf_create_pads_links_create_pads_links() - RPF pads links creation
> + * @vsp1: Pointer to VSP1 device
> + * @entity: Pointer to VSP1 entity
> + *
> + * return negative error code or zero on success
> + */
> +int vsp1_rpf_create_pads_links(struct vsp1_device *vsp1,
> +			       struct vsp1_entity *entity)
> +{
> +	struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
> +
> +	/* Connect the video device to the RPF. */
> +	return media_create_pad_link(&rpf->video.video.entity, 0,
> +				     &rpf->entity.subdev.entity,
> +				     RWPF_PAD_SINK,
> +				     MEDIA_LNK_FL_ENABLED |
> +				     MEDIA_LNK_FL_IMMUTABLE);
> +}
> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.h
> b/drivers/media/platform/vsp1/vsp1_rwpf.h index f452dce1a931..6638b3587369
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_rwpf.h
> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.h
> @@ -50,6 +50,11 @@ static inline struct vsp1_rwpf *to_rwpf(struct
> v4l2_subdev *subdev) struct vsp1_rwpf *vsp1_rpf_create(struct vsp1_device
> *vsp1, unsigned int index); struct vsp1_rwpf *vsp1_wpf_create(struct
> vsp1_device *vsp1, unsigned int index);
> 
> +int vsp1_rpf_create_pads_links(struct vsp1_device *vsp1,
> +			       struct vsp1_entity *entity);
> +int vsp1_wpf_create_pads_links(struct vsp1_device *vsp1,
> +			       struct vsp1_entity *entity);
> +
>  int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev,
>  			     struct v4l2_subdev_pad_config *cfg,
>  			     struct v4l2_subdev_mbus_code_enum *code);
> diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c
> b/drivers/media/platform/vsp1/vsp1_wpf.c index d39aa4b8aea1..1be363e4f741
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_wpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_wpf.c
> @@ -220,7 +220,6 @@ struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device
> *vsp1, unsigned int index) struct v4l2_subdev *subdev;
>  	struct vsp1_video *video;
>  	struct vsp1_rwpf *wpf;
> -	unsigned int flags;
>  	int ret;
> 
>  	wpf = devm_kzalloc(vsp1->dev, sizeof(*wpf), GFP_KERNEL);
> @@ -276,20 +275,6 @@ struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device
> *vsp1, unsigned int index) goto error;
> 
>  	wpf->entity.video = video;
> -
> -	/* Connect the video device to the WPF. All connections are immutable
> -	 * except for the WPF0 source link if a LIF is present.
> -	 */
> -	flags = MEDIA_LNK_FL_ENABLED;
> -	if (!(vsp1->pdata.features & VSP1_HAS_LIF) || index != 0)
> -		flags |= MEDIA_LNK_FL_IMMUTABLE;
> -
> -	ret = media_create_pad_link(&wpf->entity.subdev.entity,
> -				       RWPF_PAD_SOURCE,
> -				       &wpf->video.video.entity, 0, flags);
> -	if (ret < 0)
> -		goto error;
> -
>  	wpf->entity.sink = &wpf->video.video.entity;
> 
>  	return wpf;
> @@ -298,3 +283,28 @@ error:
>  	vsp1_entity_destroy(&wpf->entity);
>  	return ERR_PTR(ret);
>  }
> +
> +/*
> + * vsp1_wpf_create_pads_links_create_pads_links() - RPF pads links creation
> + * @vsp1: Pointer to VSP1 device
> + * @entity: Pointer to VSP1 entity
> + *
> + * return negative error code or zero on success
> + */
> +int vsp1_wpf_create_pads_links(struct vsp1_device *vsp1,
> +			       struct vsp1_entity *entity)
> +{
> +	struct vsp1_rwpf *wpf = to_rwpf(&entity->subdev);
> +	unsigned int flags;
> +
> +	/* Connect the video device to the WPF. All connections are immutable
> +	 * except for the WPF0 source link if a LIF is present.
> +	 */
> +	flags = MEDIA_LNK_FL_ENABLED;
> +	if (!(vsp1->pdata.features & VSP1_HAS_LIF) || entity->index != 0)
> +		flags |= MEDIA_LNK_FL_IMMUTABLE;
> +
> +	return media_create_pad_link(&wpf->entity.subdev.entity,
> +				     RWPF_PAD_SOURCE,
> +				     &wpf->video.video.entity, 0, flags);
> +}

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/5] [media] staging: omap4iss: separate links creation from entities init

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Thursday 03 September 2015 18:00:32 Javier Martinez Canillas wrote:
> The omap4iss driver initializes the entities and creates the pads links
> before the entities are registered with the media device. This does not
> work now that object IDs are used to create links so the media_device
> has to be set.
> 
> Split out the pads links creation from the entity initialization so are
> made after the entities registration.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
> 
>  drivers/staging/media/omap4iss/iss.c         | 101 +++++++++++++++---------
>  drivers/staging/media/omap4iss/iss_csi2.c    |  35 +++++++---
>  drivers/staging/media/omap4iss/iss_csi2.h    |   1 +
>  drivers/staging/media/omap4iss/iss_ipipeif.c |  29 ++++----
>  drivers/staging/media/omap4iss/iss_ipipeif.h |   1 +
>  drivers/staging/media/omap4iss/iss_resizer.c |  29 ++++----
>  drivers/staging/media/omap4iss/iss_resizer.h |   1 +
>  7 files changed, 132 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/staging/media/omap4iss/iss.c
> b/drivers/staging/media/omap4iss/iss.c index 44b88ff3ba83..076ddd412201
> 100644
> --- a/drivers/staging/media/omap4iss/iss.c
> +++ b/drivers/staging/media/omap4iss/iss.c
> @@ -1272,6 +1272,68 @@ done:
>  	return ret;
>  }
> 
> +/*
> + * iss_create_pads_links() - Pads links creation for the subdevices

Could you please s/pads_links/links/ and s/pads links/links/ ?

Apart from that,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> + * @iss : Pointer to ISS device
> + *
> + * return negative error code or zero on success
> + */
> +static int iss_create_pads_links(struct iss_device *iss)
> +{
> +	int ret;
> +
> +	ret = omap4iss_csi2_create_pads_links(iss);
> +	if (ret < 0) {
> +		dev_err(iss->dev, "CSI2 pads links creation failed\n");
> +		return ret;
> +	}
> +
> +	ret = omap4iss_ipipeif_create_pads_links(iss);
> +	if (ret < 0) {
> +		dev_err(iss->dev, "ISP IPIPEIF pads links creation failed\n");
> +		return ret;
> +	}
> +
> +	ret = omap4iss_resizer_create_pads_links(iss);
> +	if (ret < 0) {
> +		dev_err(iss->dev, "ISP RESIZER pads links creation failed\n");
> +		return ret;
> +	}
> +
> +	/* Connect the submodules. */
> +	ret = media_create_pad_link(
> +			&iss->csi2a.subdev.entity, CSI2_PAD_SOURCE,
> +			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = media_create_pad_link(
> +			&iss->csi2b.subdev.entity, CSI2_PAD_SOURCE,
> +			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = media_create_pad_link(
> +			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP,
> +			&iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = media_create_pad_link(
> +			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP,
> +			&iss->ipipe.subdev.entity, IPIPE_PAD_SINK, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = media_create_pad_link(
> +			&iss->ipipe.subdev.entity, IPIPE_PAD_SOURCE_VP,
> +			&iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +};
> +
>  static void iss_cleanup_modules(struct iss_device *iss)
>  {
>  	omap4iss_csi2_cleanup(iss);
> @@ -1314,41 +1376,8 @@ static int iss_initialize_modules(struct iss_device
> *iss) goto error_resizer;
>  	}
> 
> -	/* Connect the submodules. */
> -	ret = media_create_pad_link(
> -			&iss->csi2a.subdev.entity, CSI2_PAD_SOURCE,
> -			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0);
> -	if (ret < 0)
> -		goto error_link;
> -
> -	ret = media_create_pad_link(
> -			&iss->csi2b.subdev.entity, CSI2_PAD_SOURCE,
> -			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0);
> -	if (ret < 0)
> -		goto error_link;
> -
> -	ret = media_create_pad_link(
> -			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP,
> -			&iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0);
> -	if (ret < 0)
> -		goto error_link;
> -
> -	ret = media_create_pad_link(
> -			&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP,
> -			&iss->ipipe.subdev.entity, IPIPE_PAD_SINK, 0);
> -	if (ret < 0)
> -		goto error_link;
> -
> -	ret = media_create_pad_link(
> -			&iss->ipipe.subdev.entity, IPIPE_PAD_SOURCE_VP,
> -			&iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0);
> -	if (ret < 0)
> -		goto error_link;
> -
>  	return 0;
> 
> -error_link:
> -	omap4iss_resizer_cleanup(iss);
>  error_resizer:
>  	omap4iss_ipipe_cleanup(iss);
>  error_ipipe:
> @@ -1461,10 +1490,16 @@ static int iss_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto error_modules;
> 
> +	ret = iss_create_pads_links(iss);
> +	if (ret < 0)
> +		goto error_entities;
> +
>  	omap4iss_put(iss);
> 
>  	return 0;
> 
> +error_entities:
> +	iss_unregister_entities(iss);
>  error_modules:
>  	iss_cleanup_modules(iss);
>  error_iss:
> diff --git a/drivers/staging/media/omap4iss/iss_csi2.c
> b/drivers/staging/media/omap4iss/iss_csi2.c index
> 50a24e8e8129..907e59edcb04 100644
> --- a/drivers/staging/media/omap4iss/iss_csi2.c
> +++ b/drivers/staging/media/omap4iss/iss_csi2.c
> @@ -1295,16 +1295,8 @@ static int csi2_init_entities(struct iss_csi2_device
> *csi2, const char *subname) if (ret < 0)
>  		goto error_video;
> 
> -	/* Connect the CSI2 subdev to the video node. */
> -	ret = media_create_pad_link(&csi2->subdev.entity, CSI2_PAD_SOURCE,
> -				       &csi2->video_out.video.entity, 0, 0);
> -	if (ret < 0)
> -		goto error_link;
> -
>  	return 0;
> 
> -error_link:
> -	omap4iss_video_cleanup(&csi2->video_out);
>  error_video:
>  	media_entity_cleanup(&csi2->subdev.entity);
>  	return ret;
> @@ -1347,6 +1339,33 @@ int omap4iss_csi2_init(struct iss_device *iss)
>  }
> 
>  /*
> + * omap4iss_csi2_create_pads_links() - CSI2 pads links creation
> + * @iss: Pointer to ISS device
> + *
> + * return negative error code or zero on success
> + */
> +int omap4iss_csi2_create_pads_links(struct iss_device *iss)
> +{
> +	struct iss_csi2_device *csi2a = &iss->csi2a;
> +	struct iss_csi2_device *csi2b = &iss->csi2b;
> +	int ret;
> +
> +	/* Connect the CSI2a subdev to the video node. */
> +	ret = media_create_pad_link(&csi2a->subdev.entity, CSI2_PAD_SOURCE,
> +				    &csi2a->video_out.video.entity, 0, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Connect the CSI2b subdev to the video node. */
> +	ret = media_create_pad_link(&csi2b->subdev.entity, CSI2_PAD_SOURCE,
> +				    &csi2b->video_out.video.entity, 0, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +/*
>   * omap4iss_csi2_cleanup - Routine for module driver cleanup
>   */
>  void omap4iss_csi2_cleanup(struct iss_device *iss)
> diff --git a/drivers/staging/media/omap4iss/iss_csi2.h
> b/drivers/staging/media/omap4iss/iss_csi2.h index
> 3b37978a3bdf..ad9a02d2a576 100644
> --- a/drivers/staging/media/omap4iss/iss_csi2.h
> +++ b/drivers/staging/media/omap4iss/iss_csi2.h
> @@ -151,6 +151,7 @@ struct iss_csi2_device {
>  void omap4iss_csi2_isr(struct iss_csi2_device *csi2);
>  int omap4iss_csi2_reset(struct iss_csi2_device *csi2);
>  int omap4iss_csi2_init(struct iss_device *iss);
> +int omap4iss_csi2_create_pads_links(struct iss_device *iss);
>  void omap4iss_csi2_cleanup(struct iss_device *iss);
>  void omap4iss_csi2_unregister_entities(struct iss_csi2_device *csi2);
>  int omap4iss_csi2_register_entities(struct iss_csi2_device *csi2,
> diff --git a/drivers/staging/media/omap4iss/iss_ipipeif.c
> b/drivers/staging/media/omap4iss/iss_ipipeif.c index
> e46b2c07bd5d..36a570e60496 100644
> --- a/drivers/staging/media/omap4iss/iss_ipipeif.c
> +++ b/drivers/staging/media/omap4iss/iss_ipipeif.c
> @@ -762,18 +762,7 @@ static int ipipeif_init_entities(struct
> iss_ipipeif_device *ipipeif) ipipeif->video_out.bpl_zero_padding = 1;
>  	ipipeif->video_out.bpl_max = 0x1ffe0;
> 
> -	ret = omap4iss_video_init(&ipipeif->video_out, "ISP IPIPEIF");
> -	if (ret < 0)
> -		return ret;
> -
> -	/* Connect the IPIPEIF subdev to the video node. */
> -	ret = media_create_pad_link(&ipipeif->subdev.entity,
> -				       IPIPEIF_PAD_SOURCE_ISIF_SF,
> -				       &ipipeif->video_out.video.entity, 0, 0);
> -	if (ret < 0)
> -		return ret;
> -
> -	return 0;
> +	return omap4iss_video_init(&ipipeif->video_out, "ISP IPIPEIF");
>  }
> 
>  void omap4iss_ipipeif_unregister_entities(struct iss_ipipeif_device
> *ipipeif) @@ -826,6 +815,22 @@ int omap4iss_ipipeif_init(struct iss_device
> *iss) }
> 
>  /*
> + * omap4iss_ipipeif_create_pads_links() - IPIPEIF pads links creation
> + * @iss: Pointer to ISS device
> + *
> + * return negative error code or zero on success
> + */
> +int omap4iss_ipipeif_create_pads_links(struct iss_device *iss)
> +{
> +	struct iss_ipipeif_device *ipipeif = &iss->ipipeif;
> +
> +	/* Connect the IPIPEIF subdev to the video node. */
> +	return media_create_pad_link(&ipipeif->subdev.entity,
> +				     IPIPEIF_PAD_SOURCE_ISIF_SF,
> +				     &ipipeif->video_out.video.entity, 0, 0);
> +}
> +
> +/*
>   * omap4iss_ipipeif_cleanup - IPIPEIF module cleanup.
>   * @iss: Device pointer specific to the OMAP4 ISS.
>   */
> diff --git a/drivers/staging/media/omap4iss/iss_ipipeif.h
> b/drivers/staging/media/omap4iss/iss_ipipeif.h index
> cbdccb982eee..28357a65ae97 100644
> --- a/drivers/staging/media/omap4iss/iss_ipipeif.h
> +++ b/drivers/staging/media/omap4iss/iss_ipipeif.h
> @@ -78,6 +78,7 @@ struct iss_ipipeif_device {
>  struct iss_device;
> 
>  int omap4iss_ipipeif_init(struct iss_device *iss);
> +int omap4iss_ipipeif_create_pads_links(struct iss_device *iss);
>  void omap4iss_ipipeif_cleanup(struct iss_device *iss);
>  int omap4iss_ipipeif_register_entities(struct iss_ipipeif_device *ipipeif,
>  	struct v4l2_device *vdev);
> diff --git a/drivers/staging/media/omap4iss/iss_resizer.c
> b/drivers/staging/media/omap4iss/iss_resizer.c index
> bc5001002cc5..5de093715f98 100644
> --- a/drivers/staging/media/omap4iss/iss_resizer.c
> +++ b/drivers/staging/media/omap4iss/iss_resizer.c
> @@ -806,18 +806,7 @@ static int resizer_init_entities(struct
> iss_resizer_device *resizer) resizer->video_out.bpl_zero_padding = 1;
>  	resizer->video_out.bpl_max = 0x1ffe0;
> 
> -	ret = omap4iss_video_init(&resizer->video_out, "ISP resizer a");
> -	if (ret < 0)
> -		return ret;
> -
> -	/* Connect the RESIZER subdev to the video node. */
> -	ret = media_create_pad_link(&resizer->subdev.entity,
> -				       RESIZER_PAD_SOURCE_MEM,
> -				       &resizer->video_out.video.entity, 0, 0);
> -	if (ret < 0)
> -		return ret;
> -
> -	return 0;
> +	return omap4iss_video_init(&resizer->video_out, "ISP resizer a");
>  }
> 
>  void omap4iss_resizer_unregister_entities(struct iss_resizer_device
> *resizer) @@ -870,6 +859,22 @@ int omap4iss_resizer_init(struct iss_device
> *iss) }
> 
>  /*
> + * omap4iss_resizer_create_pads_links() - RESIZER pads links creation
> + * @iss: Pointer to ISS device
> + *
> + * return negative error code or zero on success
> + */
> +int omap4iss_resizer_create_pads_links(struct iss_device *iss)
> +{
> +	struct iss_resizer_device *resizer = &iss->resizer;
> +
> +	/* Connect the RESIZER subdev to the video node. */
> +	return media_create_pad_link(&resizer->subdev.entity,
> +				     RESIZER_PAD_SOURCE_MEM,
> +				     &resizer->video_out.video.entity, 0, 0);
> +}
> +
> +/*
>   * omap4iss_resizer_cleanup - RESIZER module cleanup.
>   * @iss: Device pointer specific to the OMAP4 ISS.
>   */
> diff --git a/drivers/staging/media/omap4iss/iss_resizer.h
> b/drivers/staging/media/omap4iss/iss_resizer.h index
> 3727498b06a3..00ecc151e511 100644
> --- a/drivers/staging/media/omap4iss/iss_resizer.h
> +++ b/drivers/staging/media/omap4iss/iss_resizer.h
> @@ -61,6 +61,7 @@ struct iss_resizer_device {
>  struct iss_device;
> 
>  int omap4iss_resizer_init(struct iss_device *iss);
> +int omap4iss_resizer_create_pads_links(struct iss_device *iss);
>  void omap4iss_resizer_cleanup(struct iss_device *iss);
>  int omap4iss_resizer_register_entities(struct iss_resizer_device *resizer,
>  	struct v4l2_device *vdev);

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/5] [media] uvcvideo: create pad links after subdev registration

[Javier Martinez Canillas]

Hello Laurent,

On 12/05/2015 11:44 PM, Laurent Pinchart wrote:
> Hi Javier,
> 
> Thank you for the patch.
>

Thanks for your review.
 
> On Monday 07 September 2015 16:10:25 Javier Martinez Canillas wrote:
> 
> [snip]
> 
>> From 8be356e77eeefdc5c0738dd429205f3398c5b76c Mon Sep 17 00:00:00 2001
>> From: Javier Martinez Canillas <javier@osg.samsung.com>
>> Date: Thu, 3 Sep 2015 13:46:06 +0200
>> Subject: [PATCH v2 4/5] [media] uvcvideo: create pad links after subdev
>>  registration
>>
>> The uvc driver creates the pads links before the media entity is
>> registered with the media device. This doesn't work now that obj
>> IDs are used to create links so the media_device has to be set.
>>
>> Move entities registration logic before pads links creation.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>> ---
>>
>> Changes since v1:
>>  - Don't try to register a UVC entity subdev for type UVC_TT_STREAMING.
>>
>>  drivers/media/usb/uvc/uvc_entity.c | 23 +++++++++++++++++++----
>>  1 file changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_entity.c
>> b/drivers/media/usb/uvc/uvc_entity.c index 429e428ccd93..7f82b65b238e
>> 100644
>> --- a/drivers/media/usb/uvc/uvc_entity.c
>> +++ b/drivers/media/usb/uvc/uvc_entity.c
>> @@ -26,6 +26,15 @@
>>  static int uvc_mc_register_entity(struct uvc_video_chain *chain,
>>         struct uvc_entity *entity)
>>  {
>> +       if (UVC_ENTITY_TYPE(entity) == UVC_TT_STREAMING)
>> +               return 0;
>> +
>> +       return v4l2_device_register_subdev(&chain->dev->vdev,
>> &entity->subdev);
>> +}
>> +
>> +static int uvc_mc_create_pads_links(struct uvc_video_chain *chain,
>> +                                   struct uvc_entity *entity)
>> +{
>>         const u32 flags = MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE;
>>         struct media_entity *sink;
>>         unsigned int i;
>> @@ -62,10 +71,7 @@ static int uvc_mc_register_entity(struct uvc_video_chain
>> *chain, return ret;
>>         }
>>  
>> -       if (UVC_ENTITY_TYPE(entity) == UVC_TT_STREAMING)
>> -               return 0;
>> -
>> -       return v4l2_device_register_subdev(&chain->dev->vdev,
>> &entity->subdev);
>> +       return 0;
>>  }
>>  
>>  static struct v4l2_subdev_ops uvc_subdev_ops = {
>> @@ -124,5 +130,14 @@ int uvc_mc_register_entities(struct uvc_video_chain
>> *chain) }
>>         }
>>  
>> +       list_for_each_entry(entity, &chain->entities, chain) {
>> +               ret = uvc_mc_create_pads_links(chain, entity);
> 
> You can call this uvc_mc_create_links(), there's no other type of links in the 
> driver.
>

Ok.
 
>> +               if (ret < 0) {
>> +                       uvc_printk(KERN_INFO, "Failed to create pads links
>> for "
> 
> Same here, I'd s/pad links/links/.
>

Ok.
 
>> +                                  "entity %u\n", entity->id);
>> +                       return ret;
>> +               }
>> +       }
> 
> This creates three loops, and I think that's one too much. The reason why init 
> and register are separate is that the latter creates links, which requires all 
> entities to be initialized. If you move link create after registration I 
> believe you can init and register in a single loop (just move the 
> v4l2_device_register_subdev() call in the appropriate location in 
> uvc_mc_init_entity()) and then create links in a second loop.
> 

You are right, I'll simplify this to only have two loops as suggested.

>>         return 0;
>>  }
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 3/5] [media] v4l: vsp1: separate links creation from entities init

[Javier Martinez Canillas]

Hello Laurent,

On 12/05/2015 11:51 PM, Laurent Pinchart wrote:
> Hi Javier,
> 
> Thank you for the patch.
>

Thanks for your feedback.
 
> On Thursday 03 September 2015 18:00:34 Javier Martinez Canillas wrote:
>> The vsp1 driver initializes the entities and creates the pads links
>> before the entities are registered with the media device. This doesn't
>> work now that object IDs are used to create links so the media_device
>> has to be set.
>>
>> Split out the pads links creation from the entity initialization so are
>> made after the entities registration.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>> ---
>>
>>  drivers/media/platform/vsp1/vsp1_drv.c  | 14 ++++++++++--
>>  drivers/media/platform/vsp1/vsp1_rpf.c  | 29 ++++++++++++++++--------
>>  drivers/media/platform/vsp1/vsp1_rwpf.h |  5 +++++
>>  drivers/media/platform/vsp1/vsp1_wpf.c  | 40 +++++++++++++++++-------------
>>  4 files changed, 62 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c
>> b/drivers/media/platform/vsp1/vsp1_drv.c index 2aa427d3ff39..8f995d267646
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_drv.c
>> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
>> @@ -260,9 +260,19 @@ static int vsp1_create_entities(struct vsp1_device
>> *vsp1)
>>
>>  	/* Create links. */
>>  	list_for_each_entry(entity, &vsp1->entities, list_dev) {
>> -		if (entity->type == VSP1_ENTITY_LIF ||
>> -		    entity->type == VSP1_ENTITY_RPF)
>> +		if (entity->type == VSP1_ENTITY_LIF) {
>> +			ret = vsp1_wpf_create_pads_links(vsp1, entity);
> 
> Could you please s/pads_links/links/ ? There's no other type of links handled 
> by the driver.
>

Sure, I'll do that for all the drivers that only handle pad links.
 
>> +			if (ret < 0)
>> +				goto done;
>> +			continue;
> 
> I would use
> 
> 	} else if (...) {
> 
> instead of a continue.
>

Yes, that will be better indeed.
 
>> +		}
>> +
>> +		if (entity->type == VSP1_ENTITY_RPF) {
>> +			ret = vsp1_rpf_create_pads_links(vsp1, entity);
>> +			if (ret < 0)
>> +				goto done;
>>  			continue;
> 
> Same here.
>

Ok.
 
> Apart from that,
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>

Thanks.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 1/5] [media] staging: omap4iss: separate links creation from entities init

[Javier Martinez Canillas]

Hello Laurent,

On 12/06/2015 12:10 AM, Laurent Pinchart wrote:
> Hi Javier,
> 
> Thank you for the patch.
>

Thanks for your feedback.
 
> On Thursday 03 September 2015 18:00:32 Javier Martinez Canillas wrote:
>> The omap4iss driver initializes the entities and creates the pads links
>> before the entities are registered with the media device. This does not
>> work now that object IDs are used to create links so the media_device
>> has to be set.
>>
>> Split out the pads links creation from the entity initialization so are
>> made after the entities registration.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>> ---
>>
>>  drivers/staging/media/omap4iss/iss.c         | 101 +++++++++++++++---------
>>  drivers/staging/media/omap4iss/iss_csi2.c    |  35 +++++++---
>>  drivers/staging/media/omap4iss/iss_csi2.h    |   1 +
>>  drivers/staging/media/omap4iss/iss_ipipeif.c |  29 ++++----
>>  drivers/staging/media/omap4iss/iss_ipipeif.h |   1 +
>>  drivers/staging/media/omap4iss/iss_resizer.c |  29 ++++----
>>  drivers/staging/media/omap4iss/iss_resizer.h |   1 +
>>  7 files changed, 132 insertions(+), 65 deletions(-)
>>
>> diff --git a/drivers/staging/media/omap4iss/iss.c
>> b/drivers/staging/media/omap4iss/iss.c index 44b88ff3ba83..076ddd412201
>> 100644
>> --- a/drivers/staging/media/omap4iss/iss.c
>> +++ b/drivers/staging/media/omap4iss/iss.c
>> @@ -1272,6 +1272,68 @@ done:
>>  	return ret;
>>  }
>>
>> +/*
>> + * iss_create_pads_links() - Pads links creation for the subdevices
> 
> Could you please s/pads_links/links/ and s/pads links/links/ ?
>

Yes, as mentioned in the other thread, I'll do that for all the
drivers that only create pad links.
 
> Apart from that,
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>

Thanks!

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America