[PATCHv3 00/10] rcar-vin: clean up and prepare for Gen3

[PATCHv3 00/10] rcar-vin: clean up and prepare for Gen3

[Niklas Söderlund]

Hi,

This series cleans up issues that have been found in the driver and 
prepares it for Gen3 support. The series is based on v4.8-rc1 and is 
tested om Koelsch. The most noteworthy fix in in the series is patch 
9/10 that fixes a of_node_put() warning during loading of the driver.

The series is the first cleanup series but all patches are marked as v3 
since they have been broken out of the Gen3 enablement series which have 
been stuck for a while. Hopefully these cleanups can be picked up and 
then I can rebase and repost all other pending rcar-vin patches ontop of 
this clean up. The changlog from the Gen3 enablement branch is however 
kept here for reference.

Changes since v2:
- Drop all Gen3 enablement patches.
- Split a large cosmetic cleanup patch in smaller ones.
- Fix comment from Sergei about return code from platform_get_irq().
- Rebase ontop of v4.8-rc1

Changes since v1:
- Address review comments from Laurent.
- Split cleanup of driver to smaller chunks.
- Remove initial work for v4l2 framework changes to support a pad aware
s_stream operation.
- Picked up patch for incorrect media bus format.
- Removed Ulrich patches which now have been picked up in media_tree.

Niklas Söderlund (10):
  [media] rcar-vin: fix indentation errors in rcar-v4l2.c
  [media] rcar-vin: reduce indentation in rvin_s_dv_timings()
  [media] rcar-vin: arrange enum chip_id in chronological order
  [media] rcar-vin: rename entity to digital
  [media] rcar-vin: return correct error from platform_get_irq()
  [media] rcar-vin: do not use v4l2_device_call_until_err()
  [media] rcar-vin: add dependency on MEDIA_CONTROLLER
  [media] rcar-vin: move chip check for pixelformat support
  [media] rcar-vin: rework how subdeivce is found and bound
  [media] rcar-vin: move media bus information to struct
    rvin_graph_entity

 drivers/media/platform/rcar-vin/Kconfig     |   2 +-
 drivers/media/platform/rcar-vin/rcar-core.c | 258 +++++++++++++---------------
 drivers/media/platform/rcar-vin/rcar-dma.c  |  18 +-
 drivers/media/platform/rcar-vin/rcar-v4l2.c |  91 +++++-----
 drivers/media/platform/rcar-vin/rcar-vin.h  |  25 +--
 5 files changed, 186 insertions(+), 208 deletions(-)

-- 
2.9.2

[PATCHv3 01/10] [media] rcar-vin: fix indentation errors in rcar-v4l2.c

[Niklas Söderlund]

Fix broken indentations and line breaks.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 46 +++++++++++++----------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 10a5c10..f26e3cd 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -93,9 +93,9 @@ static u32 rvin_format_sizeimage(struct v4l2_pix_format *pix)
  */
 
 static int __rvin_try_format_source(struct rvin_dev *vin,
-					u32 which,
-					struct v4l2_pix_format *pix,
-					struct rvin_source_fmt *source)
+				    u32 which,
+				    struct v4l2_pix_format *pix,
+				    struct rvin_source_fmt *source)
 {
 	struct v4l2_subdev *sd;
 	struct v4l2_subdev_pad_config *pad_cfg;
@@ -133,9 +133,9 @@ cleanup:
 }
 
 static int __rvin_try_format(struct rvin_dev *vin,
-				 u32 which,
-				 struct v4l2_pix_format *pix,
-				 struct rvin_source_fmt *source)
+			     u32 which,
+			     struct v4l2_pix_format *pix,
+			     struct rvin_source_fmt *source)
 {
 	const struct rvin_video_format *info;
 	u32 rwidth, rheight, walign;
@@ -219,7 +219,7 @@ static int rvin_try_fmt_vid_cap(struct file *file, void *priv,
 	struct rvin_source_fmt source;
 
 	return __rvin_try_format(vin, V4L2_SUBDEV_FORMAT_TRY, &f->fmt.pix,
-				     &source);
+				 &source);
 }
 
 static int rvin_s_fmt_vid_cap(struct file *file, void *priv,
@@ -233,7 +233,7 @@ static int rvin_s_fmt_vid_cap(struct file *file, void *priv,
 		return -EBUSY;
 
 	ret = __rvin_try_format(vin, V4L2_SUBDEV_FORMAT_ACTIVE, &f->fmt.pix,
-				    &source);
+				&source);
 	if (ret)
 		return ret;
 
@@ -334,8 +334,8 @@ static int rvin_s_selection(struct file *file, void *fh,
 		vin->crop = s->r = r;
 
 		vin_dbg(vin, "Cropped %dx%d@%d:%d of %dx%d\n",
-			 r.width, r.height, r.left, r.top,
-			 vin->source.width, vin->source.height);
+			r.width, r.height, r.left, r.top,
+			vin->source.width, vin->source.height);
 		break;
 	case V4L2_SEL_TGT_COMPOSE:
 		/* Make sure compose rect fits inside output format */
@@ -359,8 +359,8 @@ static int rvin_s_selection(struct file *file, void *fh,
 		vin->compose = s->r = r;
 
 		vin_dbg(vin, "Compose %dx%d@%d:%d in %dx%d\n",
-			 r.width, r.height, r.left, r.top,
-			 vin->format.width, vin->format.height);
+			r.width, r.height, r.left, r.top,
+			vin->format.width, vin->format.height);
 		break;
 	default:
 		return -EINVAL;
@@ -483,7 +483,7 @@ static int rvin_subscribe_event(struct v4l2_fh *fh,
 }
 
 static int rvin_enum_dv_timings(struct file *file, void *priv_fh,
-				    struct v4l2_enum_dv_timings *timings)
+				struct v4l2_enum_dv_timings *timings)
 {
 	struct rvin_dev *vin = video_drvdata(file);
 	struct v4l2_subdev *sd = vin_to_source(vin);
@@ -500,14 +500,13 @@ static int rvin_enum_dv_timings(struct file *file, void *priv_fh,
 }
 
 static int rvin_s_dv_timings(struct file *file, void *priv_fh,
-				    struct v4l2_dv_timings *timings)
+			     struct v4l2_dv_timings *timings)
 {
 	struct rvin_dev *vin = video_drvdata(file);
 	struct v4l2_subdev *sd = vin_to_source(vin);
 	int err;
 
-	err = v4l2_subdev_call(sd,
-			video, s_dv_timings, timings);
+	err = v4l2_subdev_call(sd, video, s_dv_timings, timings);
 	if (!err) {
 		vin->source.width = timings->bt.width;
 		vin->source.height = timings->bt.height;
@@ -518,27 +517,25 @@ static int rvin_s_dv_timings(struct file *file, void *priv_fh,
 }
 
 static int rvin_g_dv_timings(struct file *file, void *priv_fh,
-				    struct v4l2_dv_timings *timings)
+			     struct v4l2_dv_timings *timings)
 {
 	struct rvin_dev *vin = video_drvdata(file);
 	struct v4l2_subdev *sd = vin_to_source(vin);
 
-	return v4l2_subdev_call(sd,
-			video, g_dv_timings, timings);
+	return v4l2_subdev_call(sd, video, g_dv_timings, timings);
 }
 
 static int rvin_query_dv_timings(struct file *file, void *priv_fh,
-				    struct v4l2_dv_timings *timings)
+				 struct v4l2_dv_timings *timings)
 {
 	struct rvin_dev *vin = video_drvdata(file);
 	struct v4l2_subdev *sd = vin_to_source(vin);
 
-	return v4l2_subdev_call(sd,
-			video, query_dv_timings, timings);
+	return v4l2_subdev_call(sd, video, query_dv_timings, timings);
 }
 
 static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
-				    struct v4l2_dv_timings_cap *cap)
+			       struct v4l2_dv_timings_cap *cap)
 {
 	struct rvin_dev *vin = video_drvdata(file);
 	struct v4l2_subdev *sd = vin_to_source(vin);
@@ -825,8 +822,7 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
 	vin->src_pad_idx = 0;
 #if defined(CONFIG_MEDIA_CONTROLLER)
 	for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
-		if (sd->entity.pads[pad_idx].flags
-				== MEDIA_PAD_FL_SOURCE)
+		if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SOURCE)
 			break;
 	if (pad_idx >= sd->entity.num_pads)
 		return -EINVAL;
-- 
2.9.2

[PATCHv3 02/10] [media] rcar-vin: reduce indentation in rvin_s_dv_timings()

[Niklas Söderlund]

Align style with the rest of the driver.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index f26e3cd..72fe6bc 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -504,16 +504,18 @@ static int rvin_s_dv_timings(struct file *file, void *priv_fh,
 {
 	struct rvin_dev *vin = video_drvdata(file);
 	struct v4l2_subdev *sd = vin_to_source(vin);
-	int err;
-
-	err = v4l2_subdev_call(sd, video, s_dv_timings, timings);
-	if (!err) {
-		vin->source.width = timings->bt.width;
-		vin->source.height = timings->bt.height;
-		vin->format.width = timings->bt.width;
-		vin->format.height = timings->bt.height;
-	}
-	return err;
+	int ret;
+
+	ret = v4l2_subdev_call(sd, video, s_dv_timings, timings);
+	if (ret)
+		return ret;
+
+	vin->source.width = timings->bt.width;
+	vin->source.height = timings->bt.height;
+	vin->format.width = timings->bt.width;
+	vin->format.height = timings->bt.height;
+
+	return 0;
 }
 
 static int rvin_g_dv_timings(struct file *file, void *priv_fh,
-- 
2.9.2

[PATCHv3 03/10] [media] rcar-vin: arrange enum chip_id in chronological order

[Niklas Söderlund]

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-vin.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index 31ad39a..b9274132 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -30,9 +30,9 @@
 #define HW_BUFFER_MASK 0x7f
 
 enum chip_id {
-	RCAR_GEN2,
 	RCAR_H1,
 	RCAR_M1,
+	RCAR_GEN2,
 };
 
 /**
-- 
2.9.2

[PATCHv3 04/10] [media] rcar-vin: rename entity to digital

[Niklas Söderlund]

When Gen3 support is added to the driver more then one possible video
source entity will be possible. Knowing that the name entity is a bad
one, rename it to digital since it will deal with the digital input
source.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 46 ++++++++++++++---------------
 drivers/media/platform/rcar-vin/rcar-vin.h  |  6 ++--
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 4b2007b..a1eb26b 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -60,7 +60,7 @@ static int rvin_mbus_supported(struct rvin_dev *vin)
 	return false;
 }
 
-static int rvin_graph_notify_complete(struct v4l2_async_notifier *notifier)
+static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier)
 {
 	struct rvin_dev *vin = notifier_to_vin(notifier);
 	int ret;
@@ -79,31 +79,31 @@ static int rvin_graph_notify_complete(struct v4l2_async_notifier *notifier)
 	return rvin_v4l2_probe(vin);
 }
 
-static void rvin_graph_notify_unbind(struct v4l2_async_notifier *notifier,
-				     struct v4l2_subdev *sd,
-				     struct v4l2_async_subdev *asd)
+static void rvin_digital_notify_unbind(struct v4l2_async_notifier *notifier,
+				       struct v4l2_subdev *subdev,
+				       struct v4l2_async_subdev *asd)
 {
 	struct rvin_dev *vin = notifier_to_vin(notifier);
 
 	rvin_v4l2_remove(vin);
 }
 
-static int rvin_graph_notify_bound(struct v4l2_async_notifier *notifier,
-				   struct v4l2_subdev *subdev,
-				   struct v4l2_async_subdev *asd)
+static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
+				     struct v4l2_subdev *subdev,
+				     struct v4l2_async_subdev *asd)
 {
 	struct rvin_dev *vin = notifier_to_vin(notifier);
 
 	vin_dbg(vin, "subdev %s bound\n", subdev->name);
 
-	vin->entity.entity = &subdev->entity;
-	vin->entity.subdev = subdev;
+	vin->digital.entity = &subdev->entity;
+	vin->digital.subdev = subdev;
 
 	return 0;
 }
 
-static int rvin_graph_parse(struct rvin_dev *vin,
-			    struct device_node *node)
+static int rvin_digital_parse(struct rvin_dev *vin,
+			      struct device_node *node)
 {
 	struct device_node *remote;
 	struct device_node *ep = NULL;
@@ -131,10 +131,10 @@ static int rvin_graph_parse(struct rvin_dev *vin,
 		}
 
 		/* Remote node to connect */
-		if (!vin->entity.node) {
-			vin->entity.node = remote;
-			vin->entity.asd.match_type = V4L2_ASYNC_MATCH_OF;
-			vin->entity.asd.match.of.node = remote;
+		if (!vin->digital.node) {
+			vin->digital.node = remote;
+			vin->digital.asd.match_type = V4L2_ASYNC_MATCH_OF;
+			vin->digital.asd.match.of.node = remote;
 			ret++;
 		}
 	}
@@ -144,13 +144,13 @@ static int rvin_graph_parse(struct rvin_dev *vin,
 	return ret;
 }
 
-static int rvin_graph_init(struct rvin_dev *vin)
+static int rvin_digital_init(struct rvin_dev *vin)
 {
 	struct v4l2_async_subdev **subdevs = NULL;
 	int ret;
 
 	/* Parse the graph to extract a list of subdevice DT nodes. */
-	ret = rvin_graph_parse(vin, vin->dev->of_node);
+	ret = rvin_digital_parse(vin, vin->dev->of_node);
 	if (ret < 0) {
 		vin_err(vin, "Graph parsing failed\n");
 		goto done;
@@ -173,13 +173,13 @@ static int rvin_graph_init(struct rvin_dev *vin)
 		goto done;
 	}
 
-	subdevs[0] = &vin->entity.asd;
+	subdevs[0] = &vin->digital.asd;
 
 	vin->notifier.subdevs = subdevs;
 	vin->notifier.num_subdevs = 1;
-	vin->notifier.bound = rvin_graph_notify_bound;
-	vin->notifier.unbind = rvin_graph_notify_unbind;
-	vin->notifier.complete = rvin_graph_notify_complete;
+	vin->notifier.bound = rvin_digital_notify_bound;
+	vin->notifier.unbind = rvin_digital_notify_unbind;
+	vin->notifier.complete = rvin_digital_notify_complete;
 
 	ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
 	if (ret < 0) {
@@ -192,7 +192,7 @@ static int rvin_graph_init(struct rvin_dev *vin)
 done:
 	if (ret < 0) {
 		v4l2_async_notifier_unregister(&vin->notifier);
-		of_node_put(vin->entity.node);
+		of_node_put(vin->digital.node);
 	}
 
 	return ret;
@@ -289,7 +289,7 @@ static int rcar_vin_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = rvin_graph_init(vin);
+	ret = rvin_digital_init(vin);
 	if (ret < 0)
 		goto error;
 
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index b9274132..93daa05 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -90,7 +90,7 @@ struct rvin_graph_entity {
  * @src_pad_idx:	source pad index for media controller drivers
  * @ctrl_handler:	V4L2 control handler
  * @notifier:		V4L2 asynchronous subdevs notifier
- * @entity:		entity in the DT for subdevice
+ * @digital:		entity in the DT for local digital subdevice
  *
  * @lock:		protects @queue
  * @queue:		vb2 buffers queue
@@ -120,7 +120,7 @@ struct rvin_dev {
 	int src_pad_idx;
 	struct v4l2_ctrl_handler ctrl_handler;
 	struct v4l2_async_notifier notifier;
-	struct rvin_graph_entity entity;
+	struct rvin_graph_entity digital;
 
 	struct mutex lock;
 	struct vb2_queue queue;
@@ -139,7 +139,7 @@ struct rvin_dev {
 	struct v4l2_rect compose;
 };
 
-#define vin_to_source(vin)		vin->entity.subdev
+#define vin_to_source(vin)		vin->digital.subdev
 
 /* Debug */
 #define vin_dbg(d, fmt, arg...)		dev_dbg(d->dev, fmt, ##arg)
-- 
2.9.2

[PATCHv3 05/10] [media] rcar-vin: return correct error from platform_get_irq()

[Niklas Söderlund]

Fix a error from the original driver where the wrong error code is
returned if the driver fails to get a IRQ number from
platform_get_irq().

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index a1eb26b..3941134 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -282,8 +282,8 @@ static int rcar_vin_probe(struct platform_device *pdev)
 		return PTR_ERR(vin->base);
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0)
-		return ret;
+	if (irq < 0)
+		return irq;
 
 	ret = rvin_dma_probe(vin, irq);
 	if (ret)
-- 
2.9.2

[PATCHv3 06/10] [media] rcar-vin: do not use v4l2_device_call_until_err()

[Niklas Söderlund]

Fix a error from the original driver where v4l2_device_call_until_err()
where used for the pad specific v4l2 operation set_fmt.  Also fix up the
error path from this fix so if there is an error it will be propagated
to the caller.

The error path label have also been renamed as a result from a
nitpicking review comment since we are fixing other issues here.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 72fe6bc..3f80a0b 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -114,10 +114,9 @@ static int __rvin_try_format_source(struct rvin_dev *vin,
 
 	format.pad = vin->src_pad_idx;
 
-	ret = v4l2_device_call_until_err(sd->v4l2_dev, 0, pad, set_fmt,
-					 pad_cfg, &format);
-	if (ret < 0)
-		goto cleanup;
+	ret = v4l2_subdev_call(sd, pad, set_fmt, pad_cfg, &format);
+	if (ret < 0 && ret != -ENOIOCTLCMD)
+		goto done;
 
 	v4l2_fill_pix_format(pix, &format.format);
 
@@ -127,9 +126,9 @@ static int __rvin_try_format_source(struct rvin_dev *vin,
 	vin_dbg(vin, "Source resolution: %ux%u\n", source->width,
 		source->height);
 
-cleanup:
+done:
 	v4l2_subdev_free_pad_config(pad_cfg);
-	return 0;
+	return ret;
 }
 
 static int __rvin_try_format(struct rvin_dev *vin,
-- 
2.9.2

[PATCHv3 07/10] [media] rcar-vin: add dependency on MEDIA_CONTROLLER

[Niklas Söderlund]

This is done in preparation for Gen3 support where media controller
support will be mandatory for the driver.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/Kconfig     | 2 +-
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 7 +------
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/Kconfig b/drivers/media/platform/rcar-vin/Kconfig
index b2ff2d4..111d2a1 100644
--- a/drivers/media/platform/rcar-vin/Kconfig
+++ b/drivers/media/platform/rcar-vin/Kconfig
@@ -1,6 +1,6 @@
 config VIDEO_RCAR_VIN
 	tristate "R-Car Video Input (VIN) Driver"
-	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA
+	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA && MEDIA_CONTROLLER
 	depends on ARCH_RENESAS || COMPILE_TEST
 	select VIDEOBUF2_DMA_CONTIG
 	---help---
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 3f80a0b..09df396 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -771,10 +771,7 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
 	struct v4l2_mbus_framefmt *mf = &fmt.format;
 	struct video_device *vdev = &vin->vdev;
 	struct v4l2_subdev *sd = vin_to_source(vin);
-#if defined(CONFIG_MEDIA_CONTROLLER)
-	int pad_idx;
-#endif
-	int ret;
+	int pad_idx, ret;
 
 	v4l2_set_subdev_hostdata(sd, vin);
 
@@ -821,7 +818,6 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
 		V4L2_CAP_READWRITE;
 
 	vin->src_pad_idx = 0;
-#if defined(CONFIG_MEDIA_CONTROLLER)
 	for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
 		if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SOURCE)
 			break;
@@ -829,7 +825,6 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
 		return -EINVAL;
 
 	vin->src_pad_idx = pad_idx;
-#endif
 	fmt.pad = vin->src_pad_idx;
 
 	/* Try to improve our guess of a reasonable window format */
-- 
2.9.2

[PATCHv3 08/10] [media] rcar-vin: move chip check for pixelformat support

[Niklas Söderlund]

The check for if the specific pixelformat is supported on the current
chip should happen in VIDIOC_S_FMT and VIDIOC_TRY_FMT and not when we
try to setup the hardware for streaming.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-dma.c  | 8 +++-----
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 5 +++++
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 496aa97..3df3f0c 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -225,11 +225,9 @@ static int rvin_setup(struct rvin_dev *vin)
 		dmr = 0;
 		break;
 	case V4L2_PIX_FMT_XBGR32:
-		if (vin->chip == RCAR_GEN2 || vin->chip == RCAR_H1) {
-			dmr = VNDMR_EXRGB;
-			break;
-		}
-		/* fall through */
+		/* Note: not supported on M1 */
+		dmr = VNDMR_EXRGB;
+		break;
 	default:
 		vin_err(vin, "Invalid pixelformat (0x%x)\n",
 			vin->format.pixelformat);
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 09df396..ef3464d 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -192,6 +192,11 @@ static int __rvin_try_format(struct rvin_dev *vin,
 	pix->sizeimage = max_t(u32, pix->sizeimage,
 			       rvin_format_sizeimage(pix));
 
+	if (vin->chip == RCAR_M1 && pix->pixelformat == V4L2_PIX_FMT_XBGR32) {
+		vin_err(vin, "pixel format XBGR32 not supported on M1\n");
+		return -EINVAL;
+	}
+
 	vin_dbg(vin, "Requested %ux%u Got %ux%u bpl: %d size: %d\n",
 		rwidth, rheight, pix->width, pix->height,
 		pix->bytesperline, pix->sizeimage);
-- 
2.9.2

[PATCHv3 09/10] [media] rcar-vin: rework how subdeivce is found and bound

[Niklas Söderlund]

The original drivers code to find a subdevice by looking in the DT grpah
and how the callbacks to the v4l2 async bind framework where poorly
written. The most obvious example of badness was the duplication of data
in the struct rvin_graph_entity.

This patch removes the data duplication, simplifies the parsing of the
DT graph and add checks to the v4l2 callbacks.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 232 +++++++++++++---------------
 drivers/media/platform/rcar-vin/rcar-vin.h  |   8 +-
 2 files changed, 111 insertions(+), 129 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 3941134..39bf6fc 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -31,15 +31,13 @@
 
 #define notifier_to_vin(n) container_of(n, struct rvin_dev, notifier)
 
-static int rvin_mbus_supported(struct rvin_dev *vin)
+static bool rvin_mbus_supported(struct rvin_dev *vin)
 {
-	struct v4l2_subdev *sd;
+	struct v4l2_subdev *sd = vin->digital.subdev;
 	struct v4l2_subdev_mbus_code_enum code = {
 		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
 	};
 
-	sd = vin_to_source(vin);
-
 	code.index = 0;
 	while (!v4l2_subdev_call(sd, pad, enum_mbus_code, NULL, &code)) {
 		code.index++;
@@ -49,8 +47,6 @@ static int rvin_mbus_supported(struct rvin_dev *vin)
 		case MEDIA_BUS_FMT_YUYV10_2X10:
 		case MEDIA_BUS_FMT_RGB888_1X24:
 			vin->source.code = code.code;
-			vin_dbg(vin, "Found supported media bus format: %d\n",
-				vin->source.code);
 			return true;
 		default:
 			break;
@@ -65,17 +61,22 @@ static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier)
 	struct rvin_dev *vin = notifier_to_vin(notifier);
 	int ret;
 
+	/* Verify subdevices mbus format */
+	if (!rvin_mbus_supported(vin)) {
+		vin_err(vin, "Unsupported media bus format for %s\n",
+			vin->digital.subdev->name);
+		return -EINVAL;
+	}
+
+	vin_dbg(vin, "Found media bus format for %s: %d\n",
+		vin->digital.subdev->name, vin->source.code);
+
 	ret = v4l2_device_register_subdev_nodes(&vin->v4l2_dev);
 	if (ret < 0) {
 		vin_err(vin, "Failed to register subdev nodes\n");
 		return ret;
 	}
 
-	if (!rvin_mbus_supported(vin)) {
-		vin_err(vin, "No supported mediabus format found\n");
-		return -EINVAL;
-	}
-
 	return rvin_v4l2_probe(vin);
 }
 
@@ -85,7 +86,14 @@ static void rvin_digital_notify_unbind(struct v4l2_async_notifier *notifier,
 {
 	struct rvin_dev *vin = notifier_to_vin(notifier);
 
-	rvin_v4l2_remove(vin);
+	if (vin->digital.subdev == subdev) {
+		vin_dbg(vin, "unbind digital subdev %s\n", subdev->name);
+		rvin_v4l2_remove(vin);
+		vin->digital.subdev = NULL;
+		return;
+	}
+
+	vin_err(vin, "no entity for subdev %s to unbind\n", subdev->name);
 }
 
 static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
@@ -94,89 +102,111 @@ static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
 {
 	struct rvin_dev *vin = notifier_to_vin(notifier);
 
-	vin_dbg(vin, "subdev %s bound\n", subdev->name);
+	v4l2_set_subdev_hostdata(subdev, vin);
 
-	vin->digital.entity = &subdev->entity;
-	vin->digital.subdev = subdev;
+	if (vin->digital.asd.match.of.node == subdev->dev->of_node) {
+		vin_dbg(vin, "bound digital subdev %s\n", subdev->name);
+		vin->digital.subdev = subdev;
+		return 0;
+	}
 
-	return 0;
+	vin_err(vin, "no entity for subdev %s to bind\n", subdev->name);
+	return -EINVAL;
 }
 
-static int rvin_digital_parse(struct rvin_dev *vin,
-			      struct device_node *node)
+static int rvin_digitial_parse_v4l2(struct rvin_dev *vin,
+				    struct device_node *ep,
+				    struct v4l2_mbus_config *mbus_cfg)
 {
-	struct device_node *remote;
-	struct device_node *ep = NULL;
-	struct device_node *next;
-	int ret = 0;
-
-	while (1) {
-		next = of_graph_get_next_endpoint(node, ep);
-		if (!next)
-			break;
+	struct v4l2_of_endpoint v4l2_ep;
+	int ret;
 
-		of_node_put(ep);
-		ep = next;
+	ret = v4l2_of_parse_endpoint(ep, &v4l2_ep);
+	if (ret) {
+		vin_err(vin, "Could not parse v4l2 endpoint\n");
+		return -EINVAL;
+	}
 
-		remote = of_graph_get_remote_port_parent(ep);
-		if (!remote) {
-			ret = -EINVAL;
-			break;
-		}
+	mbus_cfg->type = v4l2_ep.bus_type;
 
-		/* Skip entities that we have already processed. */
-		if (remote == vin->dev->of_node) {
-			of_node_put(remote);
-			continue;
-		}
+	switch (mbus_cfg->type) {
+	case V4L2_MBUS_PARALLEL:
+		vin_dbg(vin, "Found PARALLEL media bus\n");
+		mbus_cfg->flags = v4l2_ep.bus.parallel.flags;
+		break;
+	case V4L2_MBUS_BT656:
+		vin_dbg(vin, "Found BT656 media bus\n");
+		mbus_cfg->flags = 0;
+		break;
+	default:
+		vin_err(vin, "Unknown media bus type\n");
+		return -EINVAL;
+	}
 
-		/* Remote node to connect */
-		if (!vin->digital.node) {
-			vin->digital.node = remote;
-			vin->digital.asd.match_type = V4L2_ASYNC_MATCH_OF;
-			vin->digital.asd.match.of.node = remote;
-			ret++;
-		}
+	return 0;
+}
+
+static int rvin_digital_graph_parse(struct rvin_dev *vin)
+{
+	struct device_node *ep, *np;
+	int ret;
+
+	vin->digital.asd.match.of.node = NULL;
+	vin->digital.subdev = NULL;
+
+	/*
+	 * Port 0 id 0 is local digital input, try to get it.
+	 * Not all instances can or will have this, that is OK
+	 */
+	ep = of_graph_get_endpoint_by_regs(vin->dev->of_node, 0, 0);
+	if (!ep)
+		return 0;
+
+	np = of_graph_get_remote_port_parent(ep);
+	if (!np) {
+		vin_err(vin, "No remote parent for digital input\n");
+		of_node_put(ep);
+		return -EINVAL;
 	}
+	of_node_put(np);
 
+	ret = rvin_digitial_parse_v4l2(vin, ep, &vin->mbus_cfg);
 	of_node_put(ep);
+	if (ret)
+		return ret;
 
-	return ret;
+	vin->digital.asd.match.of.node = np;
+	vin->digital.asd.match_type = V4L2_ASYNC_MATCH_OF;
+
+	return 0;
 }
 
-static int rvin_digital_init(struct rvin_dev *vin)
+static int rvin_digital_graph_init(struct rvin_dev *vin)
 {
 	struct v4l2_async_subdev **subdevs = NULL;
 	int ret;
 
-	/* Parse the graph to extract a list of subdevice DT nodes. */
-	ret = rvin_digital_parse(vin, vin->dev->of_node);
-	if (ret < 0) {
-		vin_err(vin, "Graph parsing failed\n");
-		goto done;
-	}
-
-	if (!ret) {
-		vin_err(vin, "No subdev found in graph\n");
-		goto done;
-	}
+	ret = rvin_digital_graph_parse(vin);
+	if (ret)
+		return ret;
 
-	if (ret != 1) {
-		vin_err(vin, "More then one subdev found in graph\n");
-		goto done;
+	if (!vin->digital.asd.match.of.node) {
+		vin_dbg(vin, "No digital subdevice found\n");
+		return -ENODEV;
 	}
 
 	/* Register the subdevices notifier. */
 	subdevs = devm_kzalloc(vin->dev, sizeof(*subdevs), GFP_KERNEL);
-	if (subdevs == NULL) {
-		ret = -ENOMEM;
-		goto done;
-	}
+	if (subdevs == NULL)
+		return -ENOMEM;
 
 	subdevs[0] = &vin->digital.asd;
 
-	vin->notifier.subdevs = subdevs;
+	vin_dbg(vin, "Found digital subdevice %s\n",
+		of_node_full_name(subdevs[0]->match.of.node));
+
 	vin->notifier.num_subdevs = 1;
+	vin->notifier.subdevs = subdevs;
 	vin->notifier.bound = rvin_digital_notify_bound;
 	vin->notifier.unbind = rvin_digital_notify_unbind;
 	vin->notifier.complete = rvin_digital_notify_complete;
@@ -184,18 +214,10 @@ static int rvin_digital_init(struct rvin_dev *vin)
 	ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
 	if (ret < 0) {
 		vin_err(vin, "Notifier registration failed\n");
-		goto done;
-	}
-
-	ret = 0;
-
-done:
-	if (ret < 0) {
-		v4l2_async_notifier_unregister(&vin->notifier);
-		of_node_put(vin->digital.node);
+		return ret;
 	}
 
-	return ret;
+	return 0;
 }
 
 /* -----------------------------------------------------------------------------
@@ -213,52 +235,9 @@ static const struct of_device_id rvin_of_id_table[] = {
 };
 MODULE_DEVICE_TABLE(of, rvin_of_id_table);
 
-static int rvin_parse_dt(struct rvin_dev *vin)
-{
-	const struct of_device_id *match;
-	struct v4l2_of_endpoint ep;
-	struct device_node *np;
-	int ret;
-
-	match = of_match_device(of_match_ptr(rvin_of_id_table), vin->dev);
-	if (!match)
-		return -ENODEV;
-
-	vin->chip = (enum chip_id)match->data;
-
-	np = of_graph_get_next_endpoint(vin->dev->of_node, NULL);
-	if (!np) {
-		vin_err(vin, "Could not find endpoint\n");
-		return -EINVAL;
-	}
-
-	ret = v4l2_of_parse_endpoint(np, &ep);
-	if (ret) {
-		vin_err(vin, "Could not parse endpoint\n");
-		return ret;
-	}
-
-	of_node_put(np);
-
-	vin->mbus_cfg.type = ep.bus_type;
-
-	switch (vin->mbus_cfg.type) {
-	case V4L2_MBUS_PARALLEL:
-		vin->mbus_cfg.flags = ep.bus.parallel.flags;
-		break;
-	case V4L2_MBUS_BT656:
-		vin->mbus_cfg.flags = 0;
-		break;
-	default:
-		vin_err(vin, "Unknown media bus type\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static int rcar_vin_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *match;
 	struct rvin_dev *vin;
 	struct resource *mem;
 	int irq, ret;
@@ -267,11 +246,12 @@ static int rcar_vin_probe(struct platform_device *pdev)
 	if (!vin)
 		return -ENOMEM;
 
-	vin->dev = &pdev->dev;
+	match = of_match_device(of_match_ptr(rvin_of_id_table), &pdev->dev);
+	if (!match)
+		return -ENODEV;
 
-	ret = rvin_parse_dt(vin);
-	if (ret)
-		return ret;
+	vin->dev = &pdev->dev;
+	vin->chip = (enum chip_id)match->data;
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (mem == NULL)
@@ -289,7 +269,7 @@ static int rcar_vin_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = rvin_digital_init(vin);
+	ret = rvin_digital_graph_init(vin);
 	if (ret < 0)
 		goto error;
 
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index 93daa05..edfe658 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -70,10 +70,12 @@ struct rvin_video_format {
 	u8 bpp;
 };
 
+/**
+ * struct rvin_graph_entity - Video endpoint from async framework
+ * @asd:	sub-device descriptor for async framework
+ * @subdev:	subdevice matched using async framework
+ */
 struct rvin_graph_entity {
-	struct device_node *node;
-	struct media_entity *entity;
-
 	struct v4l2_async_subdev asd;
 	struct v4l2_subdev *subdev;
 };
-- 
2.9.2

[PATCHv3 10/10] [media] rcar-vin: move media bus information to struct rvin_graph_entity

[Niklas Söderlund]

The primary reason for this change is to prepare for Gen3 support where
there will be more then one possible video source. Each source will have
its own media bus format and code, so it needs to be moved from the per
device structure to a structure used to represent an individual
connection to a video source.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 12 ++++++------
 drivers/media/platform/rcar-vin/rcar-dma.c  | 10 +++++-----
 drivers/media/platform/rcar-vin/rcar-v4l2.c |  2 +-
 drivers/media/platform/rcar-vin/rcar-vin.h  |  9 +++++----
 4 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 39bf6fc..64999a2 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -31,9 +31,9 @@
 
 #define notifier_to_vin(n) container_of(n, struct rvin_dev, notifier)
 
-static bool rvin_mbus_supported(struct rvin_dev *vin)
+static bool rvin_mbus_supported(struct rvin_graph_entity *entity)
 {
-	struct v4l2_subdev *sd = vin->digital.subdev;
+	struct v4l2_subdev *sd = entity->subdev;
 	struct v4l2_subdev_mbus_code_enum code = {
 		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
 	};
@@ -46,7 +46,7 @@ static bool rvin_mbus_supported(struct rvin_dev *vin)
 		case MEDIA_BUS_FMT_YUYV8_2X8:
 		case MEDIA_BUS_FMT_YUYV10_2X10:
 		case MEDIA_BUS_FMT_RGB888_1X24:
-			vin->source.code = code.code;
+			entity->code = code.code;
 			return true;
 		default:
 			break;
@@ -62,14 +62,14 @@ static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier)
 	int ret;
 
 	/* Verify subdevices mbus format */
-	if (!rvin_mbus_supported(vin)) {
+	if (!rvin_mbus_supported(&vin->digital)) {
 		vin_err(vin, "Unsupported media bus format for %s\n",
 			vin->digital.subdev->name);
 		return -EINVAL;
 	}
 
 	vin_dbg(vin, "Found media bus format for %s: %d\n",
-		vin->digital.subdev->name, vin->source.code);
+		vin->digital.subdev->name, vin->digital.code);
 
 	ret = v4l2_device_register_subdev_nodes(&vin->v4l2_dev);
 	if (ret < 0) {
@@ -170,7 +170,7 @@ static int rvin_digital_graph_parse(struct rvin_dev *vin)
 	}
 	of_node_put(np);
 
-	ret = rvin_digitial_parse_v4l2(vin, ep, &vin->mbus_cfg);
+	ret = rvin_digitial_parse_v4l2(vin, ep, &vin->digital.mbus_cfg);
 	of_node_put(ep);
 	if (ret)
 		return ret;
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 3df3f0c..46abdb0 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -163,7 +163,7 @@ static int rvin_setup(struct rvin_dev *vin)
 	/*
 	 * Input interface
 	 */
-	switch (vin->source.code) {
+	switch (vin->digital.code) {
 	case MEDIA_BUS_FMT_YUYV8_1X16:
 		/* BT.601/BT.1358 16bit YCbCr422 */
 		vnmc |= VNMC_INF_YUV16;
@@ -171,7 +171,7 @@ static int rvin_setup(struct rvin_dev *vin)
 		break;
 	case MEDIA_BUS_FMT_YUYV8_2X8:
 		/* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
-		vnmc |= vin->mbus_cfg.type == V4L2_MBUS_BT656 ?
+		vnmc |= vin->digital.mbus_cfg.type == V4L2_MBUS_BT656 ?
 			VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
 		input_is_yuv = true;
 		break;
@@ -180,7 +180,7 @@ static int rvin_setup(struct rvin_dev *vin)
 		break;
 	case MEDIA_BUS_FMT_YUYV10_2X10:
 		/* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
-		vnmc |= vin->mbus_cfg.type == V4L2_MBUS_BT656 ?
+		vnmc |= vin->digital.mbus_cfg.type == V4L2_MBUS_BT656 ?
 			VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
 		input_is_yuv = true;
 		break;
@@ -192,11 +192,11 @@ static int rvin_setup(struct rvin_dev *vin)
 	dmr2 = VNDMR2_FTEV | VNDMR2_VLV(1);
 
 	/* Hsync Signal Polarity Select */
-	if (!(vin->mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
+	if (!(vin->digital.mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
 		dmr2 |= VNDMR2_HPS;
 
 	/* Vsync Signal Polarity Select */
-	if (!(vin->mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
+	if (!(vin->digital.mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
 		dmr2 |= VNDMR2_VPS;
 
 	/*
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index ef3464d..d0e9d65 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -106,7 +106,7 @@ static int __rvin_try_format_source(struct rvin_dev *vin,
 
 	sd = vin_to_source(vin);
 
-	v4l2_fill_mbus_format(&format.format, pix, vin->source.code);
+	v4l2_fill_mbus_format(&format.format, pix, vin->digital.code);
 
 	pad_cfg = v4l2_subdev_alloc_pad_config(sd);
 	if (pad_cfg == NULL)
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index edfe658..793184d 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -50,12 +50,10 @@ enum rvin_dma_state {
 
 /**
  * struct rvin_source_fmt - Source information
- * @code:	Media bus format from source
  * @width:	Width from source
  * @height:	Height from source
  */
 struct rvin_source_fmt {
-	u32 code;
 	u32 width;
 	u32 height;
 };
@@ -74,10 +72,15 @@ struct rvin_video_format {
  * struct rvin_graph_entity - Video endpoint from async framework
  * @asd:	sub-device descriptor for async framework
  * @subdev:	subdevice matched using async framework
+ * @code:	Media bus format from source
+ * @mbus_cfg:	Media bus format from DT
  */
 struct rvin_graph_entity {
 	struct v4l2_async_subdev asd;
 	struct v4l2_subdev *subdev;
+
+	u32 code;
+	struct v4l2_mbus_config mbus_cfg;
 };
 
 /**
@@ -85,7 +88,6 @@ struct rvin_graph_entity {
  * @dev:		(OF) device
  * @base:		device I/O register space remapped to virtual memory
  * @chip:		type of VIN chip
- * @mbus_cfg		media bus configuration
  *
  * @vdev:		V4L2 video device associated with VIN
  * @v4l2_dev:		V4L2 device
@@ -115,7 +117,6 @@ struct rvin_dev {
 	struct device *dev;
 	void __iomem *base;
 	enum chip_id chip;
-	struct v4l2_mbus_config mbus_cfg;
 
 	struct video_device vdev;
 	struct v4l2_device v4l2_dev;
-- 
2.9.2

Re: [PATCHv3 04/10] [media] rcar-vin: rename entity to digital

[Sergei Shtylyov]

Hello.

On 08/15/2016 06:06 PM, Niklas Söderlund wrote:

> When Gen3 support is added to the driver more then one possible video

    s/then/than/.

> source entity will be possible. Knowing that the name entity is a bad
> one, rename it to digital since it will deal with the digital input
> source.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
[...]

    Other than that, the patch looked OK to me.

MBR, Sergei

Re: [PATCHv3 06/10] [media] rcar-vin: do not use v4l2_device_call_until_err()

[Sergei Shtylyov]

On 08/15/2016 06:06 PM, Niklas Söderlund wrote:

> Fix a error from the original driver where v4l2_device_call_until_err()
> where used for the pad specific v4l2 operation set_fmt.  Also fix up the
> error path from this fix so if there is an error it will be propagated
> to the caller.

> The error path label have also been renamed as a result from a
> nitpicking review comment since we are fixing other issues here.

    This looks like a material for the 2 or even 3 separate patches...

> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index 72fe6bc..3f80a0b 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -114,10 +114,9 @@ static int __rvin_try_format_source(struct rvin_dev *vin,
>
>  	format.pad = vin->src_pad_idx;
>
> -	ret = v4l2_device_call_until_err(sd->v4l2_dev, 0, pad, set_fmt,
> -					 pad_cfg, &format);
> -	if (ret < 0)
> -		goto cleanup;
> +	ret = v4l2_subdev_call(sd, pad, set_fmt, pad_cfg, &format);
> +	if (ret < 0 && ret != -ENOIOCTLCMD)
> +		goto done;
>
>  	v4l2_fill_pix_format(pix, &format.format);
>
> @@ -127,9 +126,9 @@ static int __rvin_try_format_source(struct rvin_dev *vin,
>  	vin_dbg(vin, "Source resolution: %ux%u\n", source->width,
>  		source->height);
>
> -cleanup:
> +done:
>  	v4l2_subdev_free_pad_config(pad_cfg);
> -	return 0;
> +	return ret;
>  }
>
>  static int __rvin_try_format(struct rvin_dev *vin,

MBR, Sergei

Re: [PATCHv3 05/10] [media] rcar-vin: return correct error from platform_get_irq()

[Sergei Shtylyov]

On 08/15/2016 06:06 PM, Niklas Söderlund wrote:

> Fix a error from the original driver where the wrong error code is
> returned if the driver fails to get a IRQ number from
> platform_get_irq().
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index a1eb26b..3941134 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -282,8 +282,8 @@ static int rcar_vin_probe(struct platform_device *pdev)
>  		return PTR_ERR(vin->base);
>
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq <= 0)
> -		return ret;
> +	if (irq < 0)

    You don't explain this change. It's OK however (my patch fixing this 
function not to return 0 on error is in GregKH's driver-core-next tree).

> +		return irq;
>
>  	ret = rvin_dma_probe(vin, irq);
>  	if (ret)

MBR, Sergei

Re: [PATCHv3 09/10] [media] rcar-vin: rework how subdeivce is found and bound

[Sergei Shtylyov]

Hello.

On 08/15/2016 06:06 PM, Niklas Söderlund wrote:

> The original drivers code to find a subdevice by looking in the DT grpah

    s/grpah/graph/.

> and how the callbacks to the v4l2 async bind framework where poorly
> written. The most obvious example of badness was the duplication of data
> in the struct rvin_graph_entity.
>
> This patch removes the data duplication, simplifies the parsing of the
> DT graph and add checks to the v4l2 callbacks.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 232 +++++++++++++---------------
>  drivers/media/platform/rcar-vin/rcar-vin.h  |   8 +-
>  2 files changed, 111 insertions(+), 129 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 3941134..39bf6fc 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
[...]
> @@ -94,89 +102,111 @@ static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
>  {
>  	struct rvin_dev *vin = notifier_to_vin(notifier);
>
> -	vin_dbg(vin, "subdev %s bound\n", subdev->name);
> +	v4l2_set_subdev_hostdata(subdev, vin);
>
> -	vin->digital.entity = &subdev->entity;
> -	vin->digital.subdev = subdev;
> +	if (vin->digital.asd.match.of.node == subdev->dev->of_node) {
> +		vin_dbg(vin, "bound digital subdev %s\n", subdev->name);
> +		vin->digital.subdev = subdev;
> +		return 0;
> +	}
>
> -	return 0;
> +	vin_err(vin, "no entity for subdev %s to bind\n", subdev->name);
> +	return -EINVAL;
>  }
>
> -static int rvin_digital_parse(struct rvin_dev *vin,
> -			      struct device_node *node)
> +static int rvin_digitial_parse_v4l2(struct rvin_dev *vin,

    s/digitial/digital/.

[...]
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> index 93daa05..edfe658 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -70,10 +70,12 @@ struct rvin_video_format {
>  	u8 bpp;
>  };
>
> +/**
> + * struct rvin_graph_entity - Video endpoint from async framework
> + * @asd:	sub-device descriptor for async framework
> + * @subdev:	subdevice matched using async framework
> + */

     Looks like a materia for a separate patch...

>  struct rvin_graph_entity {
> -	struct device_node *node;
> -	struct media_entity *entity;
> -
>  	struct v4l2_async_subdev asd;
>  	struct v4l2_subdev *subdev;
>  };

MBR, Sergei