All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/5] davinci: VPIF: add DT support
@ 2016-12-07 18:30 ` Kevin Hilman
  0 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-07 18:30 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Sakari Ailus, linux-media
  Cc: Sekhar Nori, Axel Haslam, Bartosz Gołaszewski,
	Alexandre Bailon, David Lechner, Patrick Titiano,
	linux-arm-kernel

Prepare the groundwork for adding DT support for davinci VPIF drivers.
This series does some fixups/cleanups and then adds the DT binding and
DT compatible string matching for DT probing.

The controversial part from previous versions around async subdev
parsing, and specifically hard-coding the input/output routing of
subdevs, has been left out of this series.  That part can be done as a
follow-on step after agreement has been reached on the path forward.
With this version, platforms can still use the VPIF capture/display
drivers, but must provide platform_data for the subdevs and subdev
routing.

Tested video capture to memory on da850-lcdk board using composite
input.

Changes since v5:
- locking fix: updated comment around lock variable
- binding doc: added example for 
- added reviewed-by tags from Laurent (thanks!)

Changes since v4:
- dropped controversial async subdev parsing support.  That can be
  done as a follow-up step after the discussions have finalized on the
    right approach.
    - DT binding Acked by DT maintainer (Rob H.)
    - reworked locking fix (suggested by Laurent)

Changes since v3:
- move to a single VPIF node, DT binding updated accordingly
- misc fixes/updates based on reviews from Sakari

Changes since v2:
- DT binding doc: fix example to use correct compatible

Changes since v1:
- more specific compatible strings, based on SoC: ti,da850-vpif*
- fix locking bug when unlocking over subdev s_stream


Kevin Hilman (5):
  [media] davinci: VPIF: fix module loading, init errors
  [media] davinci: vpif_capture: remove hard-coded I2C adapter id
  [media] davinci: vpif_capture: fix start/stop streaming locking
  [media] dt-bindings: add TI VPIF documentation
  [media] davinci: VPIF: add basic support for DT init

 .../devicetree/bindings/media/ti,da850-vpif.txt    | 83 ++++++++++++++++++++++
 drivers/media/platform/davinci/vpif.c              | 14 +++-
 drivers/media/platform/davinci/vpif_capture.c      | 26 +++++--
 drivers/media/platform/davinci/vpif_capture.h      |  2 +-
 drivers/media/platform/davinci/vpif_display.c      |  6 ++
 include/media/davinci/vpif_types.h                 |  1 +
 6 files changed, 125 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

-- 
2.9.3


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 0/5] davinci: VPIF: add DT support
@ 2016-12-07 18:30 ` Kevin Hilman
  0 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-07 18:30 UTC (permalink / raw)
  To: linux-arm-kernel

Prepare the groundwork for adding DT support for davinci VPIF drivers.
This series does some fixups/cleanups and then adds the DT binding and
DT compatible string matching for DT probing.

The controversial part from previous versions around async subdev
parsing, and specifically hard-coding the input/output routing of
subdevs, has been left out of this series.  That part can be done as a
follow-on step after agreement has been reached on the path forward.
With this version, platforms can still use the VPIF capture/display
drivers, but must provide platform_data for the subdevs and subdev
routing.

Tested video capture to memory on da850-lcdk board using composite
input.

Changes since v5:
- locking fix: updated comment around lock variable
- binding doc: added example for 
- added reviewed-by tags from Laurent (thanks!)

Changes since v4:
- dropped controversial async subdev parsing support.  That can be
  done as a follow-up step after the discussions have finalized on the
    right approach.
    - DT binding Acked by DT maintainer (Rob H.)
    - reworked locking fix (suggested by Laurent)

Changes since v3:
- move to a single VPIF node, DT binding updated accordingly
- misc fixes/updates based on reviews from Sakari

Changes since v2:
- DT binding doc: fix example to use correct compatible

Changes since v1:
- more specific compatible strings, based on SoC: ti,da850-vpif*
- fix locking bug when unlocking over subdev s_stream


Kevin Hilman (5):
  [media] davinci: VPIF: fix module loading, init errors
  [media] davinci: vpif_capture: remove hard-coded I2C adapter id
  [media] davinci: vpif_capture: fix start/stop streaming locking
  [media] dt-bindings: add TI VPIF documentation
  [media] davinci: VPIF: add basic support for DT init

 .../devicetree/bindings/media/ti,da850-vpif.txt    | 83 ++++++++++++++++++++++
 drivers/media/platform/davinci/vpif.c              | 14 +++-
 drivers/media/platform/davinci/vpif_capture.c      | 26 +++++--
 drivers/media/platform/davinci/vpif_capture.h      |  2 +-
 drivers/media/platform/davinci/vpif_display.c      |  6 ++
 include/media/davinci/vpif_types.h                 |  1 +
 6 files changed, 125 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

-- 
2.9.3

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 1/5] [media] davinci: VPIF: fix module loading, init errors
  2016-12-07 18:30 ` Kevin Hilman
@ 2016-12-07 18:30   ` Kevin Hilman
  -1 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-07 18:30 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Sakari Ailus, linux-media
  Cc: Sekhar Nori, Axel Haslam, Bartosz Gołaszewski,
	Alexandre Bailon, David Lechner, Patrick Titiano,
	linux-arm-kernel

Fix problems with automatic module loading by adding MODULE_ALIAS.  Also
fix various load-time errors cause by incorrect or not present
platform_data.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c         |  5 ++++-
 drivers/media/platform/davinci/vpif_capture.c | 15 ++++++++++++++-
 drivers/media/platform/davinci/vpif_display.c |  6 ++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..f50148dcba64 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -32,6 +32,9 @@
 MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
 MODULE_LICENSE("GPL");
 
+#define VPIF_DRIVER_NAME	"vpif"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
+
 #define VPIF_CH0_MAX_MODES	22
 #define VPIF_CH1_MAX_MODES	2
 #define VPIF_CH2_MAX_MODES	15
@@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
 
 static struct platform_driver vpif_driver = {
 	.driver = {
-		.name	= "vpif",
+		.name	= VPIF_DRIVER_NAME,
 		.pm	= vpif_pm_ops,
 	},
 	.remove = vpif_remove,
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..20c4344ed118 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -45,6 +45,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME	"vpif_capture"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* global variables */
 static struct vpif_device vpif_obj = { {NULL} };
@@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
 
 	vpif_dbg(2, debug, "vpif_input_to_subdev\n");
 
+	if (!chan_cfg)
+		return -1;
+	if (input_index >= chan_cfg->input_count)
+		return -1;
 	subdev_name = chan_cfg->inputs[input_index].subdev_name;
 	if (subdev_name == NULL)
 		return -1;
@@ -654,7 +659,7 @@ static int vpif_input_to_subdev(
 	/* loop through the sub device list to get the sub device info */
 	for (i = 0; i < vpif_cfg->subdev_count; i++) {
 		subdev_info = &vpif_cfg->subdev_info[i];
-		if (!strcmp(subdev_info->name, subdev_name))
+		if (subdev_info && !strcmp(subdev_info->name, subdev_name))
 			return i;
 	}
 	return -1;
@@ -685,6 +690,9 @@ static int vpif_set_input(
 	if (sd_index >= 0) {
 		sd = vpif_obj.sd[sd_index];
 		subdev_info = &vpif_cfg->subdev_info[sd_index];
+	} else {
+		/* no subdevice, no input to setup */
+		return 0;
 	}
 
 	/* first setup input path from sub device to vpif */
@@ -1435,6 +1443,11 @@ static __init int vpif_probe(struct platform_device *pdev)
 	int res_idx = 0;
 	int i, err;
 
+	if (!pdev->dev.platform_data) {
+		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
+		return -EINVAL;
+	}
+
 	vpif_dev = &pdev->dev;
 
 	err = initialize_vpif();
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 75b27233ec2f..7f632b757d32 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -42,6 +42,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME	"vpif_display"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* Is set to 1 in case of SDTV formats, 2 in case of HDTV formats. */
 static int ycmux_mode;
@@ -1249,6 +1250,11 @@ static __init int vpif_probe(struct platform_device *pdev)
 	int res_idx = 0;
 	int i, err;
 
+	if (!pdev->dev.platform_data) {
+		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
+		return -EINVAL;
+	}
+
 	vpif_dev = &pdev->dev;
 	err = initialize_vpif();
 
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v6 1/5] [media] davinci: VPIF: fix module loading, init errors
@ 2016-12-07 18:30   ` Kevin Hilman
  0 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-07 18:30 UTC (permalink / raw)
  To: linux-arm-kernel

Fix problems with automatic module loading by adding MODULE_ALIAS.  Also
fix various load-time errors cause by incorrect or not present
platform_data.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c         |  5 ++++-
 drivers/media/platform/davinci/vpif_capture.c | 15 ++++++++++++++-
 drivers/media/platform/davinci/vpif_display.c |  6 ++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..f50148dcba64 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -32,6 +32,9 @@
 MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
 MODULE_LICENSE("GPL");
 
+#define VPIF_DRIVER_NAME	"vpif"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
+
 #define VPIF_CH0_MAX_MODES	22
 #define VPIF_CH1_MAX_MODES	2
 #define VPIF_CH2_MAX_MODES	15
@@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
 
 static struct platform_driver vpif_driver = {
 	.driver = {
-		.name	= "vpif",
+		.name	= VPIF_DRIVER_NAME,
 		.pm	= vpif_pm_ops,
 	},
 	.remove = vpif_remove,
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..20c4344ed118 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -45,6 +45,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME	"vpif_capture"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* global variables */
 static struct vpif_device vpif_obj = { {NULL} };
@@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
 
 	vpif_dbg(2, debug, "vpif_input_to_subdev\n");
 
+	if (!chan_cfg)
+		return -1;
+	if (input_index >= chan_cfg->input_count)
+		return -1;
 	subdev_name = chan_cfg->inputs[input_index].subdev_name;
 	if (subdev_name == NULL)
 		return -1;
@@ -654,7 +659,7 @@ static int vpif_input_to_subdev(
 	/* loop through the sub device list to get the sub device info */
 	for (i = 0; i < vpif_cfg->subdev_count; i++) {
 		subdev_info = &vpif_cfg->subdev_info[i];
-		if (!strcmp(subdev_info->name, subdev_name))
+		if (subdev_info && !strcmp(subdev_info->name, subdev_name))
 			return i;
 	}
 	return -1;
@@ -685,6 +690,9 @@ static int vpif_set_input(
 	if (sd_index >= 0) {
 		sd = vpif_obj.sd[sd_index];
 		subdev_info = &vpif_cfg->subdev_info[sd_index];
+	} else {
+		/* no subdevice, no input to setup */
+		return 0;
 	}
 
 	/* first setup input path from sub device to vpif */
@@ -1435,6 +1443,11 @@ static __init int vpif_probe(struct platform_device *pdev)
 	int res_idx = 0;
 	int i, err;
 
+	if (!pdev->dev.platform_data) {
+		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
+		return -EINVAL;
+	}
+
 	vpif_dev = &pdev->dev;
 
 	err = initialize_vpif();
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 75b27233ec2f..7f632b757d32 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -42,6 +42,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME	"vpif_display"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* Is set to 1 in case of SDTV formats, 2 in case of HDTV formats. */
 static int ycmux_mode;
@@ -1249,6 +1250,11 @@ static __init int vpif_probe(struct platform_device *pdev)
 	int res_idx = 0;
 	int i, err;
 
+	if (!pdev->dev.platform_data) {
+		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
+		return -EINVAL;
+	}
+
 	vpif_dev = &pdev->dev;
 	err = initialize_vpif();
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v6 2/5] [media] davinci: vpif_capture: remove hard-coded I2C adapter id
  2016-12-07 18:30 ` Kevin Hilman
@ 2016-12-07 18:30   ` Kevin Hilman
  -1 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-07 18:30 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Sakari Ailus, linux-media
  Cc: Sekhar Nori, Axel Haslam, Bartosz Gołaszewski,
	Alexandre Bailon, David Lechner, Patrick Titiano,
	linux-arm-kernel

Remove hard-coded I2C adapter in favor of getting the
ID from platform_data.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 5 ++++-
 include/media/davinci/vpif_types.h            | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 20c4344ed118..c24049acd40a 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1486,7 +1486,10 @@ static __init int vpif_probe(struct platform_device *pdev)
 	}
 
 	if (!vpif_obj.config->asd_sizes) {
-		i2c_adap = i2c_get_adapter(1);
+		int i2c_id = vpif_obj.config->i2c_adapter_id;
+
+		i2c_adap = i2c_get_adapter(i2c_id);
+		WARN_ON(!i2c_adap);
 		for (i = 0; i < subdev_count; i++) {
 			subdevdata = &vpif_obj.config->subdev_info[i];
 			vpif_obj.sd[i] =
diff --git a/include/media/davinci/vpif_types.h b/include/media/davinci/vpif_types.h
index 3cb1704a0650..4282a7db99d4 100644
--- a/include/media/davinci/vpif_types.h
+++ b/include/media/davinci/vpif_types.h
@@ -82,6 +82,7 @@ struct vpif_capture_config {
 	struct vpif_capture_chan_config chan_config[VPIF_CAPTURE_MAX_CHANNELS];
 	struct vpif_subdev_info *subdev_info;
 	int subdev_count;
+	int i2c_adapter_id;
 	const char *card_name;
 	struct v4l2_async_subdev **asd;	/* Flat array, arranged in groups */
 	int *asd_sizes;		/* 0-terminated array of asd group sizes */
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v6 2/5] [media] davinci: vpif_capture: remove hard-coded I2C adapter id
@ 2016-12-07 18:30   ` Kevin Hilman
  0 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-07 18:30 UTC (permalink / raw)
  To: linux-arm-kernel

Remove hard-coded I2C adapter in favor of getting the
ID from platform_data.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 5 ++++-
 include/media/davinci/vpif_types.h            | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 20c4344ed118..c24049acd40a 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1486,7 +1486,10 @@ static __init int vpif_probe(struct platform_device *pdev)
 	}
 
 	if (!vpif_obj.config->asd_sizes) {
-		i2c_adap = i2c_get_adapter(1);
+		int i2c_id = vpif_obj.config->i2c_adapter_id;
+
+		i2c_adap = i2c_get_adapter(i2c_id);
+		WARN_ON(!i2c_adap);
 		for (i = 0; i < subdev_count; i++) {
 			subdevdata = &vpif_obj.config->subdev_info[i];
 			vpif_obj.sd[i] =
diff --git a/include/media/davinci/vpif_types.h b/include/media/davinci/vpif_types.h
index 3cb1704a0650..4282a7db99d4 100644
--- a/include/media/davinci/vpif_types.h
+++ b/include/media/davinci/vpif_types.h
@@ -82,6 +82,7 @@ struct vpif_capture_config {
 	struct vpif_capture_chan_config chan_config[VPIF_CAPTURE_MAX_CHANNELS];
 	struct vpif_subdev_info *subdev_info;
 	int subdev_count;
+	int i2c_adapter_id;
 	const char *card_name;
 	struct v4l2_async_subdev **asd;	/* Flat array, arranged in groups */
 	int *asd_sizes;		/* 0-terminated array of asd group sizes */
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v6 3/5] [media] davinci: vpif_capture: fix start/stop streaming locking
  2016-12-07 18:30 ` Kevin Hilman
@ 2016-12-07 18:30   ` Kevin Hilman
  -1 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-07 18:30 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Sakari Ailus, linux-media
  Cc: Sekhar Nori, Axel Haslam, Bartosz Gołaszewski,
	Alexandre Bailon, David Lechner, Patrick Titiano,
	linux-arm-kernel

Video capture subdevs may be over I2C and may sleep during xfer, so we
cannot do IRQ-disabled locking when calling the subdev.

The IRQ-disabled locking is meant to protect the DMA queue list
throughout the rest of the driver, so update the locking in
[start|stop]_streaming to protect just this list, and update the irqlock
comment to reflect what it actually protects.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 6 +++---
 drivers/media/platform/davinci/vpif_capture.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index c24049acd40a..f72a719efb3d 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -179,8 +179,6 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
 	unsigned long addr, flags;
 	int ret;
 
-	spin_lock_irqsave(&common->irqlock, flags);
-
 	/* Initialize field_id */
 	ch->field_id = 0;
 
@@ -211,6 +209,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
 	vpif_config_addr(ch, ret);
 
 	/* Get the next frame from the buffer queue */
+	spin_lock_irqsave(&common->irqlock, flags);
 	common->cur_frm = common->next_frm = list_entry(common->dma_queue.next,
 				    struct vpif_cap_buffer, list);
 	/* Remove buffer from the buffer queue */
@@ -244,6 +243,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
 	return 0;
 
 err:
+	spin_lock_irqsave(&common->irqlock, flags);
 	list_for_each_entry_safe(buf, tmp, &common->dma_queue, list) {
 		list_del(&buf->list);
 		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);
@@ -287,7 +287,6 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
 		vpif_dbg(1, debug, "stream off failed in subdev\n");
 
 	/* release all active buffers */
-	spin_lock_irqsave(&common->irqlock, flags);
 	if (common->cur_frm == common->next_frm) {
 		vb2_buffer_done(&common->cur_frm->vb.vb2_buf,
 				VB2_BUF_STATE_ERROR);
@@ -300,6 +299,7 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
 					VB2_BUF_STATE_ERROR);
 	}
 
+	spin_lock_irqsave(&common->irqlock, flags);
 	while (!list_empty(&common->dma_queue)) {
 		common->next_frm = list_entry(common->dma_queue.next,
 						struct vpif_cap_buffer, list);
diff --git a/drivers/media/platform/davinci/vpif_capture.h b/drivers/media/platform/davinci/vpif_capture.h
index 9e35b6771d22..1d2c052deedf 100644
--- a/drivers/media/platform/davinci/vpif_capture.h
+++ b/drivers/media/platform/davinci/vpif_capture.h
@@ -67,7 +67,7 @@ struct common_obj {
 	struct vb2_queue buffer_queue;
 	/* Queue of filled frames */
 	struct list_head dma_queue;
-	/* Used in video-buf */
+	/* Protects the dma_queue field */
 	spinlock_t irqlock;
 	/* lock used to access this structure */
 	struct mutex lock;
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v6 3/5] [media] davinci: vpif_capture: fix start/stop streaming locking
@ 2016-12-07 18:30   ` Kevin Hilman
  0 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-07 18:30 UTC (permalink / raw)
  To: linux-arm-kernel

Video capture subdevs may be over I2C and may sleep during xfer, so we
cannot do IRQ-disabled locking when calling the subdev.

The IRQ-disabled locking is meant to protect the DMA queue list
throughout the rest of the driver, so update the locking in
[start|stop]_streaming to protect just this list, and update the irqlock
comment to reflect what it actually protects.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 6 +++---
 drivers/media/platform/davinci/vpif_capture.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index c24049acd40a..f72a719efb3d 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -179,8 +179,6 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
 	unsigned long addr, flags;
 	int ret;
 
-	spin_lock_irqsave(&common->irqlock, flags);
-
 	/* Initialize field_id */
 	ch->field_id = 0;
 
@@ -211,6 +209,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
 	vpif_config_addr(ch, ret);
 
 	/* Get the next frame from the buffer queue */
+	spin_lock_irqsave(&common->irqlock, flags);
 	common->cur_frm = common->next_frm = list_entry(common->dma_queue.next,
 				    struct vpif_cap_buffer, list);
 	/* Remove buffer from the buffer queue */
@@ -244,6 +243,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
 	return 0;
 
 err:
+	spin_lock_irqsave(&common->irqlock, flags);
 	list_for_each_entry_safe(buf, tmp, &common->dma_queue, list) {
 		list_del(&buf->list);
 		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);
@@ -287,7 +287,6 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
 		vpif_dbg(1, debug, "stream off failed in subdev\n");
 
 	/* release all active buffers */
-	spin_lock_irqsave(&common->irqlock, flags);
 	if (common->cur_frm == common->next_frm) {
 		vb2_buffer_done(&common->cur_frm->vb.vb2_buf,
 				VB2_BUF_STATE_ERROR);
@@ -300,6 +299,7 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
 					VB2_BUF_STATE_ERROR);
 	}
 
+	spin_lock_irqsave(&common->irqlock, flags);
 	while (!list_empty(&common->dma_queue)) {
 		common->next_frm = list_entry(common->dma_queue.next,
 						struct vpif_cap_buffer, list);
diff --git a/drivers/media/platform/davinci/vpif_capture.h b/drivers/media/platform/davinci/vpif_capture.h
index 9e35b6771d22..1d2c052deedf 100644
--- a/drivers/media/platform/davinci/vpif_capture.h
+++ b/drivers/media/platform/davinci/vpif_capture.h
@@ -67,7 +67,7 @@ struct common_obj {
 	struct vb2_queue buffer_queue;
 	/* Queue of filled frames */
 	struct list_head dma_queue;
-	/* Used in video-buf */
+	/* Protects the dma_queue field */
 	spinlock_t irqlock;
 	/* lock used to access this structure */
 	struct mutex lock;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v6 4/5] [media] dt-bindings: add TI VPIF documentation
  2016-12-07 18:30 ` Kevin Hilman
@ 2016-12-07 18:30   ` Kevin Hilman
  -1 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-07 18:30 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Sakari Ailus, linux-media
  Cc: Sekhar Nori, Axel Haslam, Bartosz Gołaszewski,
	Alexandre Bailon, David Lechner, Patrick Titiano,
	linux-arm-kernel

Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 .../devicetree/bindings/media/ti,da850-vpif.txt    | 83 ++++++++++++++++++++++
 1 file changed, 83 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
new file mode 100644
index 000000000000..6d25d7f23d26
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
@@ -0,0 +1,83 @@
+Texas Instruments VPIF
+----------------------
+
+The TI Video Port InterFace (VPIF) is the primary component for video
+capture and display on the DA850/AM18x family of TI DaVinci/Sitara
+SoCs.
+
+TI Document reference: SPRUH82C, Chapter 35
+http://www.ti.com/lit/pdf/spruh82
+
+Required properties:
+- compatible: must be "ti,da850-vpif"
+- reg: physical base address and length of the registers set for the device;
+- interrupts: should contain IRQ line for the VPIF
+
+Video Capture:
+
+VPIF has a 16-bit parallel bus input, supporting 2 8-bit channels or a
+single 16-bit channel.  It should contain at least one port child node
+with child 'endpoint' node. Please refer to the bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example using 2 8-bit input channels, one of which is connected to an
+I2C-connected TVP5147 decoder:
+
+	vpif: vpif@217000 {
+		compatible = "ti,da850-vpif";
+		reg = <0x217000 0x1000>;
+		interrupts = <92>;
+
+		port {
+			vpif_ch0: endpoint@0 {
+				  reg = <0>;
+				  bus-width = <8>;
+				  remote-endpoint = <&composite>;
+			};
+
+			vpif_ch1: endpoint@1 {
+				  reg = <1>;
+				  bus-width = <8>;
+				  data-shift = <8>;
+			};
+		};
+	};
+
+[ ... ]
+
+&i2c0 {
+
+	tvp5147@5d {
+		compatible = "ti,tvp5147";
+		reg = <0x5d>;
+		status = "okay";
+
+		port {
+			composite: endpoint {
+				hsync-active = <1>;
+				vsync-active = <1>;
+				pclk-sample = <0>;
+
+				/* VPIF channel 0 (lower 8-bits) */
+				remote-endpoint = <&vpif_ch0>;
+				bus-width = <8>;
+			};
+		};
+	};
+};
+
+
+Alternatively, an example when the bus is configured as a single
+16-bit input (e.g. for raw-capture mode):
+
+	vpif: vpif@217000 {
+		compatible = "ti,da850-vpif";
+		reg = <0x217000 0x1000>;
+		interrupts = <92>;
+
+		port {
+			vpif_ch0: endpoint {
+				  bus-width = <16>;
+			};
+		};
+	};
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v6 4/5] [media] dt-bindings: add TI VPIF documentation
@ 2016-12-07 18:30   ` Kevin Hilman
  0 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-07 18:30 UTC (permalink / raw)
  To: linux-arm-kernel

Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 .../devicetree/bindings/media/ti,da850-vpif.txt    | 83 ++++++++++++++++++++++
 1 file changed, 83 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
new file mode 100644
index 000000000000..6d25d7f23d26
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
@@ -0,0 +1,83 @@
+Texas Instruments VPIF
+----------------------
+
+The TI Video Port InterFace (VPIF) is the primary component for video
+capture and display on the DA850/AM18x family of TI DaVinci/Sitara
+SoCs.
+
+TI Document reference: SPRUH82C, Chapter 35
+http://www.ti.com/lit/pdf/spruh82
+
+Required properties:
+- compatible: must be "ti,da850-vpif"
+- reg: physical base address and length of the registers set for the device;
+- interrupts: should contain IRQ line for the VPIF
+
+Video Capture:
+
+VPIF has a 16-bit parallel bus input, supporting 2 8-bit channels or a
+single 16-bit channel.  It should contain at least one port child node
+with child 'endpoint' node. Please refer to the bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example using 2 8-bit input channels, one of which is connected to an
+I2C-connected TVP5147 decoder:
+
+	vpif: vpif at 217000 {
+		compatible = "ti,da850-vpif";
+		reg = <0x217000 0x1000>;
+		interrupts = <92>;
+
+		port {
+			vpif_ch0: endpoint at 0 {
+				  reg = <0>;
+				  bus-width = <8>;
+				  remote-endpoint = <&composite>;
+			};
+
+			vpif_ch1: endpoint at 1 {
+				  reg = <1>;
+				  bus-width = <8>;
+				  data-shift = <8>;
+			};
+		};
+	};
+
+[ ... ]
+
+&i2c0 {
+
+	tvp5147 at 5d {
+		compatible = "ti,tvp5147";
+		reg = <0x5d>;
+		status = "okay";
+
+		port {
+			composite: endpoint {
+				hsync-active = <1>;
+				vsync-active = <1>;
+				pclk-sample = <0>;
+
+				/* VPIF channel 0 (lower 8-bits) */
+				remote-endpoint = <&vpif_ch0>;
+				bus-width = <8>;
+			};
+		};
+	};
+};
+
+
+Alternatively, an example when the bus is configured as a single
+16-bit input (e.g. for raw-capture mode):
+
+	vpif: vpif at 217000 {
+		compatible = "ti,da850-vpif";
+		reg = <0x217000 0x1000>;
+		interrupts = <92>;
+
+		port {
+			vpif_ch0: endpoint {
+				  bus-width = <16>;
+			};
+		};
+	};
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v6 5/5] [media] davinci: VPIF: add basic support for DT init
  2016-12-07 18:30 ` Kevin Hilman
@ 2016-12-07 18:30   ` Kevin Hilman
  -1 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-07 18:30 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Sakari Ailus, linux-media
  Cc: Sekhar Nori, Axel Haslam, Bartosz Gołaszewski,
	Alexandre Bailon, David Lechner, Patrick Titiano,
	linux-arm-kernel

Add basic support for initialization via DT

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index f50148dcba64..1b02a6363f77 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -467,8 +467,17 @@ static const struct dev_pm_ops vpif_pm = {
 #define vpif_pm_ops NULL
 #endif
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_of_match[] = {
+	{ .compatible = "ti,da850-vpif", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_of_match);
+#endif
+
 static struct platform_driver vpif_driver = {
 	.driver = {
+		.of_match_table = of_match_ptr(vpif_of_match),
 		.name	= VPIF_DRIVER_NAME,
 		.pm	= vpif_pm_ops,
 	},
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v6 5/5] [media] davinci: VPIF: add basic support for DT init
@ 2016-12-07 18:30   ` Kevin Hilman
  0 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-07 18:30 UTC (permalink / raw)
  To: linux-arm-kernel

Add basic support for initialization via DT

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index f50148dcba64..1b02a6363f77 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -467,8 +467,17 @@ static const struct dev_pm_ops vpif_pm = {
 #define vpif_pm_ops NULL
 #endif
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_of_match[] = {
+	{ .compatible = "ti,da850-vpif", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_of_match);
+#endif
+
 static struct platform_driver vpif_driver = {
 	.driver = {
+		.of_match_table = of_match_ptr(vpif_of_match),
 		.name	= VPIF_DRIVER_NAME,
 		.pm	= vpif_pm_ops,
 	},
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH v6 0/5] davinci: VPIF: add DT support
  2016-12-07 18:30 ` Kevin Hilman
@ 2016-12-07 20:03   ` Javier Martinez Canillas
  -1 siblings, 0 replies; 40+ messages in thread
From: Javier Martinez Canillas @ 2016-12-07 20:03 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Hans Verkuil, Laurent Pinchart, Sakari Ailus,
	Linux Media Mailing List, Sekhar Nori, Axel Haslam,
	Bartosz Gołaszewski, Alexandre Bailon, David Lechner,
	Patrick Titiano, linux-arm-kernel

Hello Kevin,

On Wed, Dec 7, 2016 at 3:30 PM, Kevin Hilman <khilman@baylibre.com> wrote:
> Prepare the groundwork for adding DT support for davinci VPIF drivers.
> This series does some fixups/cleanups and then adds the DT binding and
> DT compatible string matching for DT probing.
>
> The controversial part from previous versions around async subdev
> parsing, and specifically hard-coding the input/output routing of
> subdevs, has been left out of this series.  That part can be done as a
> follow-on step after agreement has been reached on the path forward.

I had a similar need for another board (OMAP3 IGEPv2), that has a
TVP5151 video decoder (that also supports 2 composite or 1 s-video
signal) attached to the OMAP3 ISP.

I posted some RFC patches [0] to define the input signals in the DT,
and AFAICT Laurent and Hans were not against the approach but just had
some comments on the DT binding.

Basically they wanted the ports to be directly in the tvp5150 node
instead of under a connectors sub-node [1] and to just be called just
a (input / output) port instead of a connector [2].

Unfortunately I was busy with other tasks so I couldn't res-pin the
patches, but I think you could have something similar in the DT
binding for your case and it shouldn't be hard to parse the ports /
endpoints in the driver to get that information from DT and setup the
input and output pins.

> With this version, platforms can still use the VPIF capture/display
> drivers, but must provide platform_data for the subdevs and subdev
> routing.
>

I guess DT backward compatibility isn't a big issue on this platform,
since support for the platform is quite recently and after all someone
who wants to use the vpif with current DT will need platform data and
pdata-quirks anyways. So I agree with you that the input / output
signals lookup from DT could be done as a follow-up.

[0]: https://lkml.org/lkml/2016/4/12/983
[1]: https://lkml.org/lkml/2016/4/27/678
[2]: https://lkml.org/lkml/2016/11/11/346

Best regards,
Javier

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 0/5] davinci: VPIF: add DT support
@ 2016-12-07 20:03   ` Javier Martinez Canillas
  0 siblings, 0 replies; 40+ messages in thread
From: Javier Martinez Canillas @ 2016-12-07 20:03 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Kevin,

On Wed, Dec 7, 2016 at 3:30 PM, Kevin Hilman <khilman@baylibre.com> wrote:
> Prepare the groundwork for adding DT support for davinci VPIF drivers.
> This series does some fixups/cleanups and then adds the DT binding and
> DT compatible string matching for DT probing.
>
> The controversial part from previous versions around async subdev
> parsing, and specifically hard-coding the input/output routing of
> subdevs, has been left out of this series.  That part can be done as a
> follow-on step after agreement has been reached on the path forward.

I had a similar need for another board (OMAP3 IGEPv2), that has a
TVP5151 video decoder (that also supports 2 composite or 1 s-video
signal) attached to the OMAP3 ISP.

I posted some RFC patches [0] to define the input signals in the DT,
and AFAICT Laurent and Hans were not against the approach but just had
some comments on the DT binding.

Basically they wanted the ports to be directly in the tvp5150 node
instead of under a connectors sub-node [1] and to just be called just
a (input / output) port instead of a connector [2].

Unfortunately I was busy with other tasks so I couldn't res-pin the
patches, but I think you could have something similar in the DT
binding for your case and it shouldn't be hard to parse the ports /
endpoints in the driver to get that information from DT and setup the
input and output pins.

> With this version, platforms can still use the VPIF capture/display
> drivers, but must provide platform_data for the subdevs and subdev
> routing.
>

I guess DT backward compatibility isn't a big issue on this platform,
since support for the platform is quite recently and after all someone
who wants to use the vpif with current DT will need platform data and
pdata-quirks anyways. So I agree with you that the input / output
signals lookup from DT could be done as a follow-up.

[0]: https://lkml.org/lkml/2016/4/12/983
[1]: https://lkml.org/lkml/2016/4/27/678
[2]: https://lkml.org/lkml/2016/11/11/346

Best regards,
Javier

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v6 0/5] davinci: VPIF: add DT support
  2016-12-07 20:03   ` Javier Martinez Canillas
@ 2016-12-09  0:25     ` Kevin Hilman
  -1 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-09  0:25 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Hans Verkuil, Laurent Pinchart, Sakari Ailus,
	Linux Media Mailing List, Sekhar Nori, Axel Haslam,
	Bartosz Gołaszewski, Alexandre Bailon, David Lechner,
	Patrick Titiano, linux-arm-kernel

Hi Javier,

Javier Martinez Canillas <javier@dowhile0.org> writes:

> On Wed, Dec 7, 2016 at 3:30 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>> This series does some fixups/cleanups and then adds the DT binding and
>> DT compatible string matching for DT probing.
>>
>> The controversial part from previous versions around async subdev
>> parsing, and specifically hard-coding the input/output routing of
>> subdevs, has been left out of this series.  That part can be done as a
>> follow-on step after agreement has been reached on the path forward.
>
> I had a similar need for another board (OMAP3 IGEPv2), that has a
> TVP5151 video decoder (that also supports 2 composite or 1 s-video
> signal) attached to the OMAP3 ISP.
>
> I posted some RFC patches [0] to define the input signals in the DT,
> and AFAICT Laurent and Hans were not against the approach but just had
> some comments on the DT binding.
>
> Basically they wanted the ports to be directly in the tvp5150 node
> instead of under a connectors sub-node [1] and to just be called just
> a (input / output) port instead of a connector [2].
>
> Unfortunately I was busy with other tasks so I couldn't res-pin the
> patches, but I think you could have something similar in the DT
> binding for your case and it shouldn't be hard to parse the ports /
> endpoints in the driver to get that information from DT and setup the
> input and output pins.

Thanks for pointing that out.  I did see this in Hans' reply to one of
my earlier versions.  Indeed I think this could be useful in solving my
problem.

>> With is version, platforms can still use the VPIF capture/display
>> drivers, but must provide platform_data for the subdevs and subdev
>> routing.
>>
>
> I guess DT backward compatibility isn't a big issue on this platform,
> since support for the platform is quite recently and after all someone
> who wants to use the vpif with current DT will need platform data and
> pdata-quirks anyways.

That's correct.

> So I agree with you that the input / output signals lookup from DT
> could be done as a follow-up.

Thanks. I'll happily add the input/output signals once they're agreed
upon.  In the mean time, at least we can have a usable video capture on
this platform, and it's at least a step in the right direction for DT
support.

Thanks for the review,

Kevin

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 0/5] davinci: VPIF: add DT support
@ 2016-12-09  0:25     ` Kevin Hilman
  0 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-09  0:25 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Javier,

Javier Martinez Canillas <javier@dowhile0.org> writes:

> On Wed, Dec 7, 2016 at 3:30 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>> This series does some fixups/cleanups and then adds the DT binding and
>> DT compatible string matching for DT probing.
>>
>> The controversial part from previous versions around async subdev
>> parsing, and specifically hard-coding the input/output routing of
>> subdevs, has been left out of this series.  That part can be done as a
>> follow-on step after agreement has been reached on the path forward.
>
> I had a similar need for another board (OMAP3 IGEPv2), that has a
> TVP5151 video decoder (that also supports 2 composite or 1 s-video
> signal) attached to the OMAP3 ISP.
>
> I posted some RFC patches [0] to define the input signals in the DT,
> and AFAICT Laurent and Hans were not against the approach but just had
> some comments on the DT binding.
>
> Basically they wanted the ports to be directly in the tvp5150 node
> instead of under a connectors sub-node [1] and to just be called just
> a (input / output) port instead of a connector [2].
>
> Unfortunately I was busy with other tasks so I couldn't res-pin the
> patches, but I think you could have something similar in the DT
> binding for your case and it shouldn't be hard to parse the ports /
> endpoints in the driver to get that information from DT and setup the
> input and output pins.

Thanks for pointing that out.  I did see this in Hans' reply to one of
my earlier versions.  Indeed I think this could be useful in solving my
problem.

>> With is version, platforms can still use the VPIF capture/display
>> drivers, but must provide platform_data for the subdevs and subdev
>> routing.
>>
>
> I guess DT backward compatibility isn't a big issue on this platform,
> since support for the platform is quite recently and after all someone
> who wants to use the vpif with current DT will need platform data and
> pdata-quirks anyways.

That's correct.

> So I agree with you that the input / output signals lookup from DT
> could be done as a follow-up.

Thanks. I'll happily add the input/output signals once they're agreed
upon.  In the mean time, at least we can have a usable video capture on
this platform, and it's at least a step in the right direction for DT
support.

Thanks for the review,

Kevin

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v6 1/5] [media] davinci: VPIF: fix module loading, init errors
  2016-12-07 18:30   ` Kevin Hilman
@ 2016-12-09 10:07     ` Sakari Ailus
  -1 siblings, 0 replies; 40+ messages in thread
From: Sakari Ailus @ 2016-12-09 10:07 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Hans Verkuil, Laurent Pinchart, linux-media, Sekhar Nori,
	Axel Haslam, Bartosz Gołaszewski, Alexandre Bailon,
	David Lechner, Patrick Titiano, linux-arm-kernel

On Wed, Dec 07, 2016 at 10:30:21AM -0800, Kevin Hilman wrote:
> Fix problems with automatic module loading by adding MODULE_ALIAS.  Also
> fix various load-time errors cause by incorrect or not present
> platform_data.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 1/5] [media] davinci: VPIF: fix module loading, init errors
@ 2016-12-09 10:07     ` Sakari Ailus
  0 siblings, 0 replies; 40+ messages in thread
From: Sakari Ailus @ 2016-12-09 10:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 07, 2016 at 10:30:21AM -0800, Kevin Hilman wrote:
> Fix problems with automatic module loading by adding MODULE_ALIAS.  Also
> fix various load-time errors cause by incorrect or not present
> platform_data.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus
e-mail: sakari.ailus at iki.fi	XMPP: sailus at retiisi.org.uk

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v6 2/5] [media] davinci: vpif_capture: remove hard-coded I2C adapter id
  2016-12-07 18:30   ` Kevin Hilman
@ 2016-12-09 10:20     ` Sakari Ailus
  -1 siblings, 0 replies; 40+ messages in thread
From: Sakari Ailus @ 2016-12-09 10:20 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Hans Verkuil, Laurent Pinchart, linux-media, Sekhar Nori,
	Axel Haslam, Bartosz Gołaszewski, Alexandre Bailon,
	David Lechner, Patrick Titiano, linux-arm-kernel

Hi Kevin,

On Wed, Dec 07, 2016 at 10:30:22AM -0800, Kevin Hilman wrote:
> Remove hard-coded I2C adapter in favor of getting the
> ID from platform_data.
> 
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
>  drivers/media/platform/davinci/vpif_capture.c | 5 ++++-
>  include/media/davinci/vpif_types.h            | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> index 20c4344ed118..c24049acd40a 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -1486,7 +1486,10 @@ static __init int vpif_probe(struct platform_device *pdev)
>  	}
>  
>  	if (!vpif_obj.config->asd_sizes) {
> -		i2c_adap = i2c_get_adapter(1);
> +		int i2c_id = vpif_obj.config->i2c_adapter_id;

Is there a particular reason to use a temporary variable just once? I'd use
the i2c_adapter_field directly instead. Up to you.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

> +
> +		i2c_adap = i2c_get_adapter(i2c_id);
> +		WARN_ON(!i2c_adap);
>  		for (i = 0; i < subdev_count; i++) {
>  			subdevdata = &vpif_obj.config->subdev_info[i];
>  			vpif_obj.sd[i] =
> diff --git a/include/media/davinci/vpif_types.h b/include/media/davinci/vpif_types.h
> index 3cb1704a0650..4282a7db99d4 100644
> --- a/include/media/davinci/vpif_types.h
> +++ b/include/media/davinci/vpif_types.h
> @@ -82,6 +82,7 @@ struct vpif_capture_config {
>  	struct vpif_capture_chan_config chan_config[VPIF_CAPTURE_MAX_CHANNELS];
>  	struct vpif_subdev_info *subdev_info;
>  	int subdev_count;
> +	int i2c_adapter_id;
>  	const char *card_name;
>  	struct v4l2_async_subdev **asd;	/* Flat array, arranged in groups */
>  	int *asd_sizes;		/* 0-terminated array of asd group sizes */

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 2/5] [media] davinci: vpif_capture: remove hard-coded I2C adapter id
@ 2016-12-09 10:20     ` Sakari Ailus
  0 siblings, 0 replies; 40+ messages in thread
From: Sakari Ailus @ 2016-12-09 10:20 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Kevin,

On Wed, Dec 07, 2016 at 10:30:22AM -0800, Kevin Hilman wrote:
> Remove hard-coded I2C adapter in favor of getting the
> ID from platform_data.
> 
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
>  drivers/media/platform/davinci/vpif_capture.c | 5 ++++-
>  include/media/davinci/vpif_types.h            | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> index 20c4344ed118..c24049acd40a 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -1486,7 +1486,10 @@ static __init int vpif_probe(struct platform_device *pdev)
>  	}
>  
>  	if (!vpif_obj.config->asd_sizes) {
> -		i2c_adap = i2c_get_adapter(1);
> +		int i2c_id = vpif_obj.config->i2c_adapter_id;

Is there a particular reason to use a temporary variable just once? I'd use
the i2c_adapter_field directly instead. Up to you.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

> +
> +		i2c_adap = i2c_get_adapter(i2c_id);
> +		WARN_ON(!i2c_adap);
>  		for (i = 0; i < subdev_count; i++) {
>  			subdevdata = &vpif_obj.config->subdev_info[i];
>  			vpif_obj.sd[i] =
> diff --git a/include/media/davinci/vpif_types.h b/include/media/davinci/vpif_types.h
> index 3cb1704a0650..4282a7db99d4 100644
> --- a/include/media/davinci/vpif_types.h
> +++ b/include/media/davinci/vpif_types.h
> @@ -82,6 +82,7 @@ struct vpif_capture_config {
>  	struct vpif_capture_chan_config chan_config[VPIF_CAPTURE_MAX_CHANNELS];
>  	struct vpif_subdev_info *subdev_info;
>  	int subdev_count;
> +	int i2c_adapter_id;
>  	const char *card_name;
>  	struct v4l2_async_subdev **asd;	/* Flat array, arranged in groups */
>  	int *asd_sizes;		/* 0-terminated array of asd group sizes */

-- 
Sakari Ailus
e-mail: sakari.ailus at iki.fi	XMPP: sailus at retiisi.org.uk

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v6 1/5] [media] davinci: VPIF: fix module loading, init errors
  2016-12-07 18:30   ` Kevin Hilman
@ 2016-12-16  9:44     ` Hans Verkuil
  -1 siblings, 0 replies; 40+ messages in thread
From: Hans Verkuil @ 2016-12-16  9:44 UTC (permalink / raw)
  To: Kevin Hilman, Laurent Pinchart, Sakari Ailus, linux-media
  Cc: Sekhar Nori, Axel Haslam, Bartosz Gołaszewski,
	Alexandre Bailon, David Lechner, Patrick Titiano,
	linux-arm-kernel

On 07/12/16 19:30, Kevin Hilman wrote:
> Fix problems with automatic module loading by adding MODULE_ALIAS.  Also
> fix various load-time errors cause by incorrect or not present
> platform_data.
>
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
>  drivers/media/platform/davinci/vpif.c         |  5 ++++-
>  drivers/media/platform/davinci/vpif_capture.c | 15 ++++++++++++++-
>  drivers/media/platform/davinci/vpif_display.c |  6 ++++++
>  3 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
> index 0380cf2e5775..f50148dcba64 100644
> --- a/drivers/media/platform/davinci/vpif.c
> +++ b/drivers/media/platform/davinci/vpif.c
> @@ -32,6 +32,9 @@
>  MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
>  MODULE_LICENSE("GPL");
>
> +#define VPIF_DRIVER_NAME	"vpif"
> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
> +
>  #define VPIF_CH0_MAX_MODES	22
>  #define VPIF_CH1_MAX_MODES	2
>  #define VPIF_CH2_MAX_MODES	15
> @@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
>
>  static struct platform_driver vpif_driver = {
>  	.driver = {
> -		.name	= "vpif",
> +		.name	= VPIF_DRIVER_NAME,
>  		.pm	= vpif_pm_ops,
>  	},
>  	.remove = vpif_remove,
> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> index 5104cc0ee40e..20c4344ed118 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -45,6 +45,7 @@ module_param(debug, int, 0644);
>  MODULE_PARM_DESC(debug, "Debug level 0-1");
>
>  #define VPIF_DRIVER_NAME	"vpif_capture"
> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>
>  /* global variables */
>  static struct vpif_device vpif_obj = { {NULL} };
> @@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
>
>  	vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>
> +	if (!chan_cfg)
> +		return -1;
> +	if (input_index >= chan_cfg->input_count)
> +		return -1;
>  	subdev_name = chan_cfg->inputs[input_index].subdev_name;
>  	if (subdev_name == NULL)
>  		return -1;
> @@ -654,7 +659,7 @@ static int vpif_input_to_subdev(
>  	/* loop through the sub device list to get the sub device info */
>  	for (i = 0; i < vpif_cfg->subdev_count; i++) {
>  		subdev_info = &vpif_cfg->subdev_info[i];
> -		if (!strcmp(subdev_info->name, subdev_name))
> +		if (subdev_info && !strcmp(subdev_info->name, subdev_name))

Why this change? subdev_info can never be NULL.

Regards,

	Hans

>  			return i;
>  	}
>  	return -1;
> @@ -685,6 +690,9 @@ static int vpif_set_input(
>  	if (sd_index >= 0) {
>  		sd = vpif_obj.sd[sd_index];
>  		subdev_info = &vpif_cfg->subdev_info[sd_index];
> +	} else {
> +		/* no subdevice, no input to setup */
> +		return 0;
>  	}
>
>  	/* first setup input path from sub device to vpif */
> @@ -1435,6 +1443,11 @@ static __init int vpif_probe(struct platform_device *pdev)
>  	int res_idx = 0;
>  	int i, err;
>
> +	if (!pdev->dev.platform_data) {
> +		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
> +		return -EINVAL;
> +	}
> +
>  	vpif_dev = &pdev->dev;
>
>  	err = initialize_vpif();
> diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
> index 75b27233ec2f..7f632b757d32 100644
> --- a/drivers/media/platform/davinci/vpif_display.c
> +++ b/drivers/media/platform/davinci/vpif_display.c
> @@ -42,6 +42,7 @@ module_param(debug, int, 0644);
>  MODULE_PARM_DESC(debug, "Debug level 0-1");
>
>  #define VPIF_DRIVER_NAME	"vpif_display"
> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>
>  /* Is set to 1 in case of SDTV formats, 2 in case of HDTV formats. */
>  static int ycmux_mode;
> @@ -1249,6 +1250,11 @@ static __init int vpif_probe(struct platform_device *pdev)
>  	int res_idx = 0;
>  	int i, err;
>
> +	if (!pdev->dev.platform_data) {
> +		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
> +		return -EINVAL;
> +	}
> +
>  	vpif_dev = &pdev->dev;
>  	err = initialize_vpif();
>
>


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 1/5] [media] davinci: VPIF: fix module loading, init errors
@ 2016-12-16  9:44     ` Hans Verkuil
  0 siblings, 0 replies; 40+ messages in thread
From: Hans Verkuil @ 2016-12-16  9:44 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/12/16 19:30, Kevin Hilman wrote:
> Fix problems with automatic module loading by adding MODULE_ALIAS.  Also
> fix various load-time errors cause by incorrect or not present
> platform_data.
>
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
>  drivers/media/platform/davinci/vpif.c         |  5 ++++-
>  drivers/media/platform/davinci/vpif_capture.c | 15 ++++++++++++++-
>  drivers/media/platform/davinci/vpif_display.c |  6 ++++++
>  3 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
> index 0380cf2e5775..f50148dcba64 100644
> --- a/drivers/media/platform/davinci/vpif.c
> +++ b/drivers/media/platform/davinci/vpif.c
> @@ -32,6 +32,9 @@
>  MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
>  MODULE_LICENSE("GPL");
>
> +#define VPIF_DRIVER_NAME	"vpif"
> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
> +
>  #define VPIF_CH0_MAX_MODES	22
>  #define VPIF_CH1_MAX_MODES	2
>  #define VPIF_CH2_MAX_MODES	15
> @@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
>
>  static struct platform_driver vpif_driver = {
>  	.driver = {
> -		.name	= "vpif",
> +		.name	= VPIF_DRIVER_NAME,
>  		.pm	= vpif_pm_ops,
>  	},
>  	.remove = vpif_remove,
> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> index 5104cc0ee40e..20c4344ed118 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -45,6 +45,7 @@ module_param(debug, int, 0644);
>  MODULE_PARM_DESC(debug, "Debug level 0-1");
>
>  #define VPIF_DRIVER_NAME	"vpif_capture"
> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>
>  /* global variables */
>  static struct vpif_device vpif_obj = { {NULL} };
> @@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
>
>  	vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>
> +	if (!chan_cfg)
> +		return -1;
> +	if (input_index >= chan_cfg->input_count)
> +		return -1;
>  	subdev_name = chan_cfg->inputs[input_index].subdev_name;
>  	if (subdev_name == NULL)
>  		return -1;
> @@ -654,7 +659,7 @@ static int vpif_input_to_subdev(
>  	/* loop through the sub device list to get the sub device info */
>  	for (i = 0; i < vpif_cfg->subdev_count; i++) {
>  		subdev_info = &vpif_cfg->subdev_info[i];
> -		if (!strcmp(subdev_info->name, subdev_name))
> +		if (subdev_info && !strcmp(subdev_info->name, subdev_name))

Why this change? subdev_info can never be NULL.

Regards,

	Hans

>  			return i;
>  	}
>  	return -1;
> @@ -685,6 +690,9 @@ static int vpif_set_input(
>  	if (sd_index >= 0) {
>  		sd = vpif_obj.sd[sd_index];
>  		subdev_info = &vpif_cfg->subdev_info[sd_index];
> +	} else {
> +		/* no subdevice, no input to setup */
> +		return 0;
>  	}
>
>  	/* first setup input path from sub device to vpif */
> @@ -1435,6 +1443,11 @@ static __init int vpif_probe(struct platform_device *pdev)
>  	int res_idx = 0;
>  	int i, err;
>
> +	if (!pdev->dev.platform_data) {
> +		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
> +		return -EINVAL;
> +	}
> +
>  	vpif_dev = &pdev->dev;
>
>  	err = initialize_vpif();
> diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
> index 75b27233ec2f..7f632b757d32 100644
> --- a/drivers/media/platform/davinci/vpif_display.c
> +++ b/drivers/media/platform/davinci/vpif_display.c
> @@ -42,6 +42,7 @@ module_param(debug, int, 0644);
>  MODULE_PARM_DESC(debug, "Debug level 0-1");
>
>  #define VPIF_DRIVER_NAME	"vpif_display"
> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>
>  /* Is set to 1 in case of SDTV formats, 2 in case of HDTV formats. */
>  static int ycmux_mode;
> @@ -1249,6 +1250,11 @@ static __init int vpif_probe(struct platform_device *pdev)
>  	int res_idx = 0;
>  	int i, err;
>
> +	if (!pdev->dev.platform_data) {
> +		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
> +		return -EINVAL;
> +	}
> +
>  	vpif_dev = &pdev->dev;
>  	err = initialize_vpif();
>
>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v6 0/5] davinci: VPIF: add DT support
  2016-12-07 18:30 ` Kevin Hilman
@ 2016-12-16  9:47   ` Hans Verkuil
  -1 siblings, 0 replies; 40+ messages in thread
From: Hans Verkuil @ 2016-12-16  9:47 UTC (permalink / raw)
  To: Kevin Hilman, Laurent Pinchart, Sakari Ailus, linux-media
  Cc: Sekhar Nori, Axel Haslam, Bartosz Gołaszewski,
	Alexandre Bailon, David Lechner, Patrick Titiano,
	linux-arm-kernel

On 07/12/16 19:30, Kevin Hilman wrote:
> Prepare the groundwork for adding DT support for davinci VPIF drivers.
> This series does some fixups/cleanups and then adds the DT binding and
> DT compatible string matching for DT probing.
>
> The controversial part from previous versions around async subdev
> parsing, and specifically hard-coding the input/output routing of
> subdevs, has been left out of this series.  That part can be done as a
> follow-on step after agreement has been reached on the path forward.
> With this version, platforms can still use the VPIF capture/display
> drivers, but must provide platform_data for the subdevs and subdev
> routing.
>
> Tested video capture to memory on da850-lcdk board using composite
> input.

Other than the comment for the first patch this series looks good.

So once that's addressed I'll queue it up for 4.11.

Regards,

	Hans

>
> Changes since v5:
> - locking fix: updated comment around lock variable
> - binding doc: added example for
> - added reviewed-by tags from Laurent (thanks!)
>
> Changes since v4:
> - dropped controversial async subdev parsing support.  That can be
>   done as a follow-up step after the discussions have finalized on the
>     right approach.
>     - DT binding Acked by DT maintainer (Rob H.)
>     - reworked locking fix (suggested by Laurent)
>
> Changes since v3:
> - move to a single VPIF node, DT binding updated accordingly
> - misc fixes/updates based on reviews from Sakari
>
> Changes since v2:
> - DT binding doc: fix example to use correct compatible
>
> Changes since v1:
> - more specific compatible strings, based on SoC: ti,da850-vpif*
> - fix locking bug when unlocking over subdev s_stream
>
>
> Kevin Hilman (5):
>   [media] davinci: VPIF: fix module loading, init errors
>   [media] davinci: vpif_capture: remove hard-coded I2C adapter id
>   [media] davinci: vpif_capture: fix start/stop streaming locking
>   [media] dt-bindings: add TI VPIF documentation
>   [media] davinci: VPIF: add basic support for DT init
>
>  .../devicetree/bindings/media/ti,da850-vpif.txt    | 83 ++++++++++++++++++++++
>  drivers/media/platform/davinci/vpif.c              | 14 +++-
>  drivers/media/platform/davinci/vpif_capture.c      | 26 +++++--
>  drivers/media/platform/davinci/vpif_capture.h      |  2 +-
>  drivers/media/platform/davinci/vpif_display.c      |  6 ++
>  include/media/davinci/vpif_types.h                 |  1 +
>  6 files changed, 125 insertions(+), 7 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 0/5] davinci: VPIF: add DT support
@ 2016-12-16  9:47   ` Hans Verkuil
  0 siblings, 0 replies; 40+ messages in thread
From: Hans Verkuil @ 2016-12-16  9:47 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/12/16 19:30, Kevin Hilman wrote:
> Prepare the groundwork for adding DT support for davinci VPIF drivers.
> This series does some fixups/cleanups and then adds the DT binding and
> DT compatible string matching for DT probing.
>
> The controversial part from previous versions around async subdev
> parsing, and specifically hard-coding the input/output routing of
> subdevs, has been left out of this series.  That part can be done as a
> follow-on step after agreement has been reached on the path forward.
> With this version, platforms can still use the VPIF capture/display
> drivers, but must provide platform_data for the subdevs and subdev
> routing.
>
> Tested video capture to memory on da850-lcdk board using composite
> input.

Other than the comment for the first patch this series looks good.

So once that's addressed I'll queue it up for 4.11.

Regards,

	Hans

>
> Changes since v5:
> - locking fix: updated comment around lock variable
> - binding doc: added example for
> - added reviewed-by tags from Laurent (thanks!)
>
> Changes since v4:
> - dropped controversial async subdev parsing support.  That can be
>   done as a follow-up step after the discussions have finalized on the
>     right approach.
>     - DT binding Acked by DT maintainer (Rob H.)
>     - reworked locking fix (suggested by Laurent)
>
> Changes since v3:
> - move to a single VPIF node, DT binding updated accordingly
> - misc fixes/updates based on reviews from Sakari
>
> Changes since v2:
> - DT binding doc: fix example to use correct compatible
>
> Changes since v1:
> - more specific compatible strings, based on SoC: ti,da850-vpif*
> - fix locking bug when unlocking over subdev s_stream
>
>
> Kevin Hilman (5):
>   [media] davinci: VPIF: fix module loading, init errors
>   [media] davinci: vpif_capture: remove hard-coded I2C adapter id
>   [media] davinci: vpif_capture: fix start/stop streaming locking
>   [media] dt-bindings: add TI VPIF documentation
>   [media] davinci: VPIF: add basic support for DT init
>
>  .../devicetree/bindings/media/ti,da850-vpif.txt    | 83 ++++++++++++++++++++++
>  drivers/media/platform/davinci/vpif.c              | 14 +++-
>  drivers/media/platform/davinci/vpif_capture.c      | 26 +++++--
>  drivers/media/platform/davinci/vpif_capture.h      |  2 +-
>  drivers/media/platform/davinci/vpif_display.c      |  6 ++
>  include/media/davinci/vpif_types.h                 |  1 +
>  6 files changed, 125 insertions(+), 7 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v6 1/5] [media] davinci: VPIF: fix module loading, init errors
  2016-12-16  9:44     ` Hans Verkuil
@ 2016-12-16 23:39       ` Kevin Hilman
  -1 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-16 23:39 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Laurent Pinchart, Sakari Ailus, linux-media, Sekhar Nori,
	Axel Haslam, Bartosz Gołaszewski, Alexandre Bailon,
	David Lechner, Patrick Titiano, linux-arm-kernel

Hans Verkuil <hverkuil@xs4all.nl> writes:

> On 07/12/16 19:30, Kevin Hilman wrote:
>> Fix problems with automatic module loading by adding MODULE_ALIAS.  Also
>> fix various load-time errors cause by incorrect or not present
>> platform_data.
>>
>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>> ---
>>  drivers/media/platform/davinci/vpif.c         |  5 ++++-
>>  drivers/media/platform/davinci/vpif_capture.c | 15 ++++++++++++++-
>>  drivers/media/platform/davinci/vpif_display.c |  6 ++++++
>>  3 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
>> index 0380cf2e5775..f50148dcba64 100644
>> --- a/drivers/media/platform/davinci/vpif.c
>> +++ b/drivers/media/platform/davinci/vpif.c
>> @@ -32,6 +32,9 @@
>>  MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
>>  MODULE_LICENSE("GPL");
>>
>> +#define VPIF_DRIVER_NAME	"vpif"
>> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>> +
>>  #define VPIF_CH0_MAX_MODES	22
>>  #define VPIF_CH1_MAX_MODES	2
>>  #define VPIF_CH2_MAX_MODES	15
>> @@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
>>
>>  static struct platform_driver vpif_driver = {
>>  	.driver = {
>> -		.name	= "vpif",
>> +		.name	= VPIF_DRIVER_NAME,
>>  		.pm	= vpif_pm_ops,
>>  	},
>>  	.remove = vpif_remove,
>> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
>> index 5104cc0ee40e..20c4344ed118 100644
>> --- a/drivers/media/platform/davinci/vpif_capture.c
>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> @@ -45,6 +45,7 @@ module_param(debug, int, 0644);
>>  MODULE_PARM_DESC(debug, "Debug level 0-1");
>>
>>  #define VPIF_DRIVER_NAME	"vpif_capture"
>> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>>
>>  /* global variables */
>>  static struct vpif_device vpif_obj = { {NULL} };
>> @@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
>>
>>  	vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>>
>> +	if (!chan_cfg)
>> +		return -1;
>> +	if (input_index >= chan_cfg->input_count)
>> +		return -1;
>>  	subdev_name = chan_cfg->inputs[input_index].subdev_name;
>>  	if (subdev_name == NULL)
>>  		return -1;
>> @@ -654,7 +659,7 @@ static int vpif_input_to_subdev(
>>  	/* loop through the sub device list to get the sub device info */
>>  	for (i = 0; i < vpif_cfg->subdev_count; i++) {
>>  		subdev_info = &vpif_cfg->subdev_info[i];
>> -		if (!strcmp(subdev_info->name, subdev_name))
>> +		if (subdev_info && !strcmp(subdev_info->name, subdev_name))
>
> Why this change? subdev_info can never be NULL.

A debugging leftover I guess.  Will remove and resend.

Thanks for the review,

Kevin

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 1/5] [media] davinci: VPIF: fix module loading, init errors
@ 2016-12-16 23:39       ` Kevin Hilman
  0 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-16 23:39 UTC (permalink / raw)
  To: linux-arm-kernel

Hans Verkuil <hverkuil@xs4all.nl> writes:

> On 07/12/16 19:30, Kevin Hilman wrote:
>> Fix problems with automatic module loading by adding MODULE_ALIAS.  Also
>> fix various load-time errors cause by incorrect or not present
>> platform_data.
>>
>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>> ---
>>  drivers/media/platform/davinci/vpif.c         |  5 ++++-
>>  drivers/media/platform/davinci/vpif_capture.c | 15 ++++++++++++++-
>>  drivers/media/platform/davinci/vpif_display.c |  6 ++++++
>>  3 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
>> index 0380cf2e5775..f50148dcba64 100644
>> --- a/drivers/media/platform/davinci/vpif.c
>> +++ b/drivers/media/platform/davinci/vpif.c
>> @@ -32,6 +32,9 @@
>>  MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
>>  MODULE_LICENSE("GPL");
>>
>> +#define VPIF_DRIVER_NAME	"vpif"
>> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>> +
>>  #define VPIF_CH0_MAX_MODES	22
>>  #define VPIF_CH1_MAX_MODES	2
>>  #define VPIF_CH2_MAX_MODES	15
>> @@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
>>
>>  static struct platform_driver vpif_driver = {
>>  	.driver = {
>> -		.name	= "vpif",
>> +		.name	= VPIF_DRIVER_NAME,
>>  		.pm	= vpif_pm_ops,
>>  	},
>>  	.remove = vpif_remove,
>> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
>> index 5104cc0ee40e..20c4344ed118 100644
>> --- a/drivers/media/platform/davinci/vpif_capture.c
>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> @@ -45,6 +45,7 @@ module_param(debug, int, 0644);
>>  MODULE_PARM_DESC(debug, "Debug level 0-1");
>>
>>  #define VPIF_DRIVER_NAME	"vpif_capture"
>> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>>
>>  /* global variables */
>>  static struct vpif_device vpif_obj = { {NULL} };
>> @@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
>>
>>  	vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>>
>> +	if (!chan_cfg)
>> +		return -1;
>> +	if (input_index >= chan_cfg->input_count)
>> +		return -1;
>>  	subdev_name = chan_cfg->inputs[input_index].subdev_name;
>>  	if (subdev_name == NULL)
>>  		return -1;
>> @@ -654,7 +659,7 @@ static int vpif_input_to_subdev(
>>  	/* loop through the sub device list to get the sub device info */
>>  	for (i = 0; i < vpif_cfg->subdev_count; i++) {
>>  		subdev_info = &vpif_cfg->subdev_info[i];
>> -		if (!strcmp(subdev_info->name, subdev_name))
>> +		if (subdev_info && !strcmp(subdev_info->name, subdev_name))
>
> Why this change? subdev_info can never be NULL.

A debugging leftover I guess.  Will remove and resend.

Thanks for the review,

Kevin

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6.1 1/5] [media] davinci: VPIF: fix module loading, init errors
  2016-12-07 18:30   ` Kevin Hilman
@ 2016-12-17  0:47     ` Kevin Hilman
  -1 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-17  0:47 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Sakari Ailus
  Cc: linux-media, Sekhar Nori, linux-arm-kernel

Fix problems with automatic module loading by adding MODULE_ALIAS.  Also
fix various load-time errors cause by incorrect or not present
platform_data.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
Minor tweaks since v6
- added ack from Sakari
- droped an extraneous change for NULL subdev_info

 drivers/media/platform/davinci/vpif.c         |  5 ++++-
 drivers/media/platform/davinci/vpif_capture.c | 13 +++++++++++++
 drivers/media/platform/davinci/vpif_display.c |  6 ++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..f50148dcba64 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -32,6 +32,9 @@
 MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
 MODULE_LICENSE("GPL");
 
+#define VPIF_DRIVER_NAME	"vpif"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
+
 #define VPIF_CH0_MAX_MODES	22
 #define VPIF_CH1_MAX_MODES	2
 #define VPIF_CH2_MAX_MODES	15
@@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
 
 static struct platform_driver vpif_driver = {
 	.driver = {
-		.name	= "vpif",
+		.name	= VPIF_DRIVER_NAME,
 		.pm	= vpif_pm_ops,
 	},
 	.remove = vpif_remove,
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..892a26f3c5b4 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -45,6 +45,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME	"vpif_capture"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* global variables */
 static struct vpif_device vpif_obj = { {NULL} };
@@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
 
 	vpif_dbg(2, debug, "vpif_input_to_subdev\n");
 
+	if (!chan_cfg)
+		return -1;
+	if (input_index >= chan_cfg->input_count)
+		return -1;
 	subdev_name = chan_cfg->inputs[input_index].subdev_name;
 	if (subdev_name == NULL)
 		return -1;
@@ -685,6 +690,9 @@ static int vpif_set_input(
 	if (sd_index >= 0) {
 		sd = vpif_obj.sd[sd_index];
 		subdev_info = &vpif_cfg->subdev_info[sd_index];
+	} else {
+		/* no subdevice, no input to setup */
+		return 0;
 	}
 
 	/* first setup input path from sub device to vpif */
@@ -1435,6 +1443,11 @@ static __init int vpif_probe(struct platform_device *pdev)
 	int res_idx = 0;
 	int i, err;
 
+	if (!pdev->dev.platform_data) {
+		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
+		return -EINVAL;
+	}
+
 	vpif_dev = &pdev->dev;
 
 	err = initialize_vpif();
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 75b27233ec2f..7f632b757d32 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -42,6 +42,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME	"vpif_display"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* Is set to 1 in case of SDTV formats, 2 in case of HDTV formats. */
 static int ycmux_mode;
@@ -1249,6 +1250,11 @@ static __init int vpif_probe(struct platform_device *pdev)
 	int res_idx = 0;
 	int i, err;
 
+	if (!pdev->dev.platform_data) {
+		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
+		return -EINVAL;
+	}
+
 	vpif_dev = &pdev->dev;
 	err = initialize_vpif();
 
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v6.1 1/5] [media] davinci: VPIF: fix module loading, init errors
@ 2016-12-17  0:47     ` Kevin Hilman
  0 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-17  0:47 UTC (permalink / raw)
  To: linux-arm-kernel

Fix problems with automatic module loading by adding MODULE_ALIAS.  Also
fix various load-time errors cause by incorrect or not present
platform_data.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
Minor tweaks since v6
- added ack from Sakari
- droped an extraneous change for NULL subdev_info

 drivers/media/platform/davinci/vpif.c         |  5 ++++-
 drivers/media/platform/davinci/vpif_capture.c | 13 +++++++++++++
 drivers/media/platform/davinci/vpif_display.c |  6 ++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..f50148dcba64 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -32,6 +32,9 @@
 MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
 MODULE_LICENSE("GPL");
 
+#define VPIF_DRIVER_NAME	"vpif"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
+
 #define VPIF_CH0_MAX_MODES	22
 #define VPIF_CH1_MAX_MODES	2
 #define VPIF_CH2_MAX_MODES	15
@@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
 
 static struct platform_driver vpif_driver = {
 	.driver = {
-		.name	= "vpif",
+		.name	= VPIF_DRIVER_NAME,
 		.pm	= vpif_pm_ops,
 	},
 	.remove = vpif_remove,
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..892a26f3c5b4 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -45,6 +45,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME	"vpif_capture"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* global variables */
 static struct vpif_device vpif_obj = { {NULL} };
@@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
 
 	vpif_dbg(2, debug, "vpif_input_to_subdev\n");
 
+	if (!chan_cfg)
+		return -1;
+	if (input_index >= chan_cfg->input_count)
+		return -1;
 	subdev_name = chan_cfg->inputs[input_index].subdev_name;
 	if (subdev_name == NULL)
 		return -1;
@@ -685,6 +690,9 @@ static int vpif_set_input(
 	if (sd_index >= 0) {
 		sd = vpif_obj.sd[sd_index];
 		subdev_info = &vpif_cfg->subdev_info[sd_index];
+	} else {
+		/* no subdevice, no input to setup */
+		return 0;
 	}
 
 	/* first setup input path from sub device to vpif */
@@ -1435,6 +1443,11 @@ static __init int vpif_probe(struct platform_device *pdev)
 	int res_idx = 0;
 	int i, err;
 
+	if (!pdev->dev.platform_data) {
+		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
+		return -EINVAL;
+	}
+
 	vpif_dev = &pdev->dev;
 
 	err = initialize_vpif();
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 75b27233ec2f..7f632b757d32 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -42,6 +42,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME	"vpif_display"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* Is set to 1 in case of SDTV formats, 2 in case of HDTV formats. */
 static int ycmux_mode;
@@ -1249,6 +1250,11 @@ static __init int vpif_probe(struct platform_device *pdev)
 	int res_idx = 0;
 	int i, err;
 
+	if (!pdev->dev.platform_data) {
+		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
+		return -EINVAL;
+	}
+
 	vpif_dev = &pdev->dev;
 	err = initialize_vpif();
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH v6 0/5] davinci: VPIF: add DT support
  2016-12-16  9:47   ` Hans Verkuil
@ 2016-12-17  0:49     ` Kevin Hilman
  -1 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-17  0:49 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Laurent Pinchart, Sakari Ailus, linux-media, Sekhar Nori,
	Axel Haslam, Bartosz Gołaszewski, Alexandre Bailon,
	David Lechner, Patrick Titiano, linux-arm-kernel

Hans Verkuil <hverkuil@xs4all.nl> writes:

> On 07/12/16 19:30, Kevin Hilman wrote:
>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>> This series does some fixups/cleanups and then adds the DT binding and
>> DT compatible string matching for DT probing.
>>
>> The controversial part from previous versions around async subdev
>> parsing, and specifically hard-coding the input/output routing of
>> subdevs, has been left out of this series.  That part can be done as a
>> follow-on step after agreement has been reached on the path forward.
>> With this version, platforms can still use the VPIF capture/display
>> drivers, but must provide platform_data for the subdevs and subdev
>> routing.
>>
>> Tested video capture to memory on da850-lcdk board using composite
>> input.
>
> Other than the comment for the first patch this series looks good.
>
> So once that's addressed I'll queue it up for 4.11.

I've fixed that issue, and sent an update for just that patch in reply
to the original.

Thanks for the review,

Kevin

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 0/5] davinci: VPIF: add DT support
@ 2016-12-17  0:49     ` Kevin Hilman
  0 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2016-12-17  0:49 UTC (permalink / raw)
  To: linux-arm-kernel

Hans Verkuil <hverkuil@xs4all.nl> writes:

> On 07/12/16 19:30, Kevin Hilman wrote:
>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>> This series does some fixups/cleanups and then adds the DT binding and
>> DT compatible string matching for DT probing.
>>
>> The controversial part from previous versions around async subdev
>> parsing, and specifically hard-coding the input/output routing of
>> subdevs, has been left out of this series.  That part can be done as a
>> follow-on step after agreement has been reached on the path forward.
>> With this version, platforms can still use the VPIF capture/display
>> drivers, but must provide platform_data for the subdevs and subdev
>> routing.
>>
>> Tested video capture to memory on da850-lcdk board using composite
>> input.
>
> Other than the comment for the first patch this series looks good.
>
> So once that's addressed I'll queue it up for 4.11.

I've fixed that issue, and sent an update for just that patch in reply
to the original.

Thanks for the review,

Kevin

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v6 0/5] davinci: VPIF: add DT support
  2016-12-16  9:47   ` Hans Verkuil
@ 2017-01-03  9:03     ` Sekhar Nori
  -1 siblings, 0 replies; 40+ messages in thread
From: Sekhar Nori @ 2017-01-03  9:03 UTC (permalink / raw)
  To: Hans Verkuil, Kevin Hilman, Laurent Pinchart, Sakari Ailus, linux-media
  Cc: Axel Haslam, Bartosz Gołaszewski, Alexandre Bailon,
	David Lechner, Patrick Titiano, linux-arm-kernel

Hi Hans,

On Friday 16 December 2016 03:17 PM, Hans Verkuil wrote:
> On 07/12/16 19:30, Kevin Hilman wrote:
>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>> This series does some fixups/cleanups and then adds the DT binding and
>> DT compatible string matching for DT probing.
>>
>> The controversial part from previous versions around async subdev
>> parsing, and specifically hard-coding the input/output routing of
>> subdevs, has been left out of this series.  That part can be done as a
>> follow-on step after agreement has been reached on the path forward.
>> With this version, platforms can still use the VPIF capture/display
>> drivers, but must provide platform_data for the subdevs and subdev
>> routing.
>>
>> Tested video capture to memory on da850-lcdk board using composite
>> input.
> 
> Other than the comment for the first patch this series looks good.
> 
> So once that's addressed I'll queue it up for 4.11.

Can you provide an immutable commit (as it will reach v4.11) with with
this series applied? I have some platform changes to queue for v4.11
that depend on the driver updates.

Thanks,
Sekhar

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 0/5] davinci: VPIF: add DT support
@ 2017-01-03  9:03     ` Sekhar Nori
  0 siblings, 0 replies; 40+ messages in thread
From: Sekhar Nori @ 2017-01-03  9:03 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Hans,

On Friday 16 December 2016 03:17 PM, Hans Verkuil wrote:
> On 07/12/16 19:30, Kevin Hilman wrote:
>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>> This series does some fixups/cleanups and then adds the DT binding and
>> DT compatible string matching for DT probing.
>>
>> The controversial part from previous versions around async subdev
>> parsing, and specifically hard-coding the input/output routing of
>> subdevs, has been left out of this series.  That part can be done as a
>> follow-on step after agreement has been reached on the path forward.
>> With this version, platforms can still use the VPIF capture/display
>> drivers, but must provide platform_data for the subdevs and subdev
>> routing.
>>
>> Tested video capture to memory on da850-lcdk board using composite
>> input.
> 
> Other than the comment for the first patch this series looks good.
> 
> So once that's addressed I'll queue it up for 4.11.

Can you provide an immutable commit (as it will reach v4.11) with with
this series applied? I have some platform changes to queue for v4.11
that depend on the driver updates.

Thanks,
Sekhar

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v6 0/5] davinci: VPIF: add DT support
  2017-01-03  9:03     ` Sekhar Nori
@ 2017-01-03  9:12       ` Laurent Pinchart
  -1 siblings, 0 replies; 40+ messages in thread
From: Laurent Pinchart @ 2017-01-03  9:12 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: Hans Verkuil, Kevin Hilman, Sakari Ailus, linux-media,
	Axel Haslam, Bartosz Gołaszewski, Alexandre Bailon,
	David Lechner, Patrick Titiano, linux-arm-kernel

Hi Sekhar,

On Tuesday 03 Jan 2017 14:33:00 Sekhar Nori wrote:
> On Friday 16 December 2016 03:17 PM, Hans Verkuil wrote:
> > On 07/12/16 19:30, Kevin Hilman wrote:
> >> Prepare the groundwork for adding DT support for davinci VPIF drivers.
> >> This series does some fixups/cleanups and then adds the DT binding and
> >> DT compatible string matching for DT probing.
> >> 
> >> The controversial part from previous versions around async subdev
> >> parsing, and specifically hard-coding the input/output routing of
> >> subdevs, has been left out of this series.  That part can be done as a
> >> follow-on step after agreement has been reached on the path forward.
> >> With this version, platforms can still use the VPIF capture/display
> >> drivers, but must provide platform_data for the subdevs and subdev
> >> routing.
> >> 
> >> Tested video capture to memory on da850-lcdk board using composite
> >> input.
> > 
> > Other than the comment for the first patch this series looks good.
> > 
> > So once that's addressed I'll queue it up for 4.11.
> 
> Can you provide an immutable commit (as it will reach v4.11) with with
> this series applied? I have some platform changes to queue for v4.11
> that depend on the driver updates.

I don't think that's possible, given that Mauro rewrites all patches when 
handling pull requests to prepend [media] to the subject line and to add his 
SoB. Only Mauro can thus provide a stable branch, Hans can't.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 0/5] davinci: VPIF: add DT support
@ 2017-01-03  9:12       ` Laurent Pinchart
  0 siblings, 0 replies; 40+ messages in thread
From: Laurent Pinchart @ 2017-01-03  9:12 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Sekhar,

On Tuesday 03 Jan 2017 14:33:00 Sekhar Nori wrote:
> On Friday 16 December 2016 03:17 PM, Hans Verkuil wrote:
> > On 07/12/16 19:30, Kevin Hilman wrote:
> >> Prepare the groundwork for adding DT support for davinci VPIF drivers.
> >> This series does some fixups/cleanups and then adds the DT binding and
> >> DT compatible string matching for DT probing.
> >> 
> >> The controversial part from previous versions around async subdev
> >> parsing, and specifically hard-coding the input/output routing of
> >> subdevs, has been left out of this series.  That part can be done as a
> >> follow-on step after agreement has been reached on the path forward.
> >> With this version, platforms can still use the VPIF capture/display
> >> drivers, but must provide platform_data for the subdevs and subdev
> >> routing.
> >> 
> >> Tested video capture to memory on da850-lcdk board using composite
> >> input.
> > 
> > Other than the comment for the first patch this series looks good.
> > 
> > So once that's addressed I'll queue it up for 4.11.
> 
> Can you provide an immutable commit (as it will reach v4.11) with with
> this series applied? I have some platform changes to queue for v4.11
> that depend on the driver updates.

I don't think that's possible, given that Mauro rewrites all patches when 
handling pull requests to prepend [media] to the subject line and to add his 
SoB. Only Mauro can thus provide a stable branch, Hans can't.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v6 0/5] davinci: VPIF: add DT support
  2017-01-03  9:12       ` Laurent Pinchart
@ 2017-01-04 11:32         ` Sekhar Nori
  -1 siblings, 0 replies; 40+ messages in thread
From: Sekhar Nori @ 2017-01-04 11:32 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Laurent Pinchart, Hans Verkuil, Kevin Hilman, Sakari Ailus,
	linux-media, Axel Haslam, Bartosz Gołaszewski,
	Alexandre Bailon, David Lechner, Patrick Titiano,
	linux-arm-kernel

+ Mauro

On Tuesday 03 January 2017 02:42 PM, Laurent Pinchart wrote:
> Hi Sekhar,
> 
> On Tuesday 03 Jan 2017 14:33:00 Sekhar Nori wrote:
>> On Friday 16 December 2016 03:17 PM, Hans Verkuil wrote:
>>> On 07/12/16 19:30, Kevin Hilman wrote:
>>>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>>>> This series does some fixups/cleanups and then adds the DT binding and
>>>> DT compatible string matching for DT probing.
>>>>
>>>> The controversial part from previous versions around async subdev
>>>> parsing, and specifically hard-coding the input/output routing of
>>>> subdevs, has been left out of this series.  That part can be done as a
>>>> follow-on step after agreement has been reached on the path forward.
>>>> With this version, platforms can still use the VPIF capture/display
>>>> drivers, but must provide platform_data for the subdevs and subdev
>>>> routing.
>>>>
>>>> Tested video capture to memory on da850-lcdk board using composite
>>>> input.
>>>
>>> Other than the comment for the first patch this series looks good.
>>>
>>> So once that's addressed I'll queue it up for 4.11.
>>
>> Can you provide an immutable commit (as it will reach v4.11) with with
>> this series applied? I have some platform changes to queue for v4.11
>> that depend on the driver updates.
> 
> I don't think that's possible, given that Mauro rewrites all patches when 
> handling pull requests to prepend [media] to the subject line and to add his 
> SoB. Only Mauro can thus provide a stable branch, Hans can't.

Hi Mauro, once Hans sends you these patches, can you host these patches
on a stable branch, which I can merge into my pull request to ARM-SoC. I
have some platform updates that depend on these driver changes.

Ideally the branch has only these patches over an early v4.10-rc so I
include as little of media stuff as possible in my pull request.

Thanks,
Sekhar


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 0/5] davinci: VPIF: add DT support
@ 2017-01-04 11:32         ` Sekhar Nori
  0 siblings, 0 replies; 40+ messages in thread
From: Sekhar Nori @ 2017-01-04 11:32 UTC (permalink / raw)
  To: linux-arm-kernel

+ Mauro

On Tuesday 03 January 2017 02:42 PM, Laurent Pinchart wrote:
> Hi Sekhar,
> 
> On Tuesday 03 Jan 2017 14:33:00 Sekhar Nori wrote:
>> On Friday 16 December 2016 03:17 PM, Hans Verkuil wrote:
>>> On 07/12/16 19:30, Kevin Hilman wrote:
>>>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>>>> This series does some fixups/cleanups and then adds the DT binding and
>>>> DT compatible string matching for DT probing.
>>>>
>>>> The controversial part from previous versions around async subdev
>>>> parsing, and specifically hard-coding the input/output routing of
>>>> subdevs, has been left out of this series.  That part can be done as a
>>>> follow-on step after agreement has been reached on the path forward.
>>>> With this version, platforms can still use the VPIF capture/display
>>>> drivers, but must provide platform_data for the subdevs and subdev
>>>> routing.
>>>>
>>>> Tested video capture to memory on da850-lcdk board using composite
>>>> input.
>>>
>>> Other than the comment for the first patch this series looks good.
>>>
>>> So once that's addressed I'll queue it up for 4.11.
>>
>> Can you provide an immutable commit (as it will reach v4.11) with with
>> this series applied? I have some platform changes to queue for v4.11
>> that depend on the driver updates.
> 
> I don't think that's possible, given that Mauro rewrites all patches when 
> handling pull requests to prepend [media] to the subject line and to add his 
> SoB. Only Mauro can thus provide a stable branch, Hans can't.

Hi Mauro, once Hans sends you these patches, can you host these patches
on a stable branch, which I can merge into my pull request to ARM-SoC. I
have some platform updates that depend on these driver changes.

Ideally the branch has only these patches over an early v4.10-rc so I
include as little of media stuff as possible in my pull request.

Thanks,
Sekhar

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v6 0/5] davinci: VPIF: add DT support
  2016-12-17  0:49     ` Kevin Hilman
@ 2017-01-27 17:22       ` Kevin Hilman
  -1 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2017-01-27 17:22 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Laurent Pinchart, Sakari Ailus, Linux Media Mailing List,
	Sekhar Nori, Axel Haslam, Bartosz Gołaszewski,
	Alexandre Bailon, David Lechner, Patrick Titiano,
	linux-arm-kernel

On Fri, Dec 16, 2016 at 4:49 PM, Kevin Hilman <khilman@baylibre.com> wrote:
> Hans Verkuil <hverkuil@xs4all.nl> writes:
>
>> On 07/12/16 19:30, Kevin Hilman wrote:
>>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>>> This series does some fixups/cleanups and then adds the DT binding and
>>> DT compatible string matching for DT probing.
>>>
>>> The controversial part from previous versions around async subdev
>>> parsing, and specifically hard-coding the input/output routing of
>>> subdevs, has been left out of this series.  That part can be done as a
>>> follow-on step after agreement has been reached on the path forward.
>>> With this version, platforms can still use the VPIF capture/display
>>> drivers, but must provide platform_data for the subdevs and subdev
>>> routing.
>>>
>>> Tested video capture to memory on da850-lcdk board using composite
>>> input.
>>
>> Other than the comment for the first patch this series looks good.
>>
>> So once that's addressed I'll queue it up for 4.11.
>
> I've fixed that issue, and sent an update for just that patch in reply
> to the original.
>
> Thanks for the review,

Gentle ping on this series.

I'm still not seeing this series yet in linux-next, so am worried it
might not make it for v4.11.

Kevin

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 0/5] davinci: VPIF: add DT support
@ 2017-01-27 17:22       ` Kevin Hilman
  0 siblings, 0 replies; 40+ messages in thread
From: Kevin Hilman @ 2017-01-27 17:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Dec 16, 2016 at 4:49 PM, Kevin Hilman <khilman@baylibre.com> wrote:
> Hans Verkuil <hverkuil@xs4all.nl> writes:
>
>> On 07/12/16 19:30, Kevin Hilman wrote:
>>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>>> This series does some fixups/cleanups and then adds the DT binding and
>>> DT compatible string matching for DT probing.
>>>
>>> The controversial part from previous versions around async subdev
>>> parsing, and specifically hard-coding the input/output routing of
>>> subdevs, has been left out of this series.  That part can be done as a
>>> follow-on step after agreement has been reached on the path forward.
>>> With this version, platforms can still use the VPIF capture/display
>>> drivers, but must provide platform_data for the subdevs and subdev
>>> routing.
>>>
>>> Tested video capture to memory on da850-lcdk board using composite
>>> input.
>>
>> Other than the comment for the first patch this series looks good.
>>
>> So once that's addressed I'll queue it up for 4.11.
>
> I've fixed that issue, and sent an update for just that patch in reply
> to the original.
>
> Thanks for the review,

Gentle ping on this series.

I'm still not seeing this series yet in linux-next, so am worried it
might not make it for v4.11.

Kevin

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v6 0/5] davinci: VPIF: add DT support
  2017-01-27 17:22       ` Kevin Hilman
@ 2017-01-30  9:33         ` Hans Verkuil
  -1 siblings, 0 replies; 40+ messages in thread
From: Hans Verkuil @ 2017-01-30  9:33 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Laurent Pinchart, Sakari Ailus, Linux Media Mailing List,
	Sekhar Nori, Axel Haslam, Bartosz Gołaszewski,
	Alexandre Bailon, David Lechner, Patrick Titiano,
	linux-arm-kernel

On 27/01/17 18:22, Kevin Hilman wrote:
> On Fri, Dec 16, 2016 at 4:49 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>> Hans Verkuil <hverkuil@xs4all.nl> writes:
>>
>>> On 07/12/16 19:30, Kevin Hilman wrote:
>>>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>>>> This series does some fixups/cleanups and then adds the DT binding and
>>>> DT compatible string matching for DT probing.
>>>>
>>>> The controversial part from previous versions around async subdev
>>>> parsing, and specifically hard-coding the input/output routing of
>>>> subdevs, has been left out of this series.  That part can be done as a
>>>> follow-on step after agreement has been reached on the path forward.
>>>> With this version, platforms can still use the VPIF capture/display
>>>> drivers, but must provide platform_data for the subdevs and subdev
>>>> routing.
>>>>
>>>> Tested video capture to memory on da850-lcdk board using composite
>>>> input.
>>>
>>> Other than the comment for the first patch this series looks good.
>>>
>>> So once that's addressed I'll queue it up for 4.11.
>>
>> I've fixed that issue, and sent an update for just that patch in reply
>> to the original.
>>
>> Thanks for the review,
>
> Gentle ping on this series.
>
> I'm still not seeing this series yet in linux-next, so am worried it
> might not make it for v4.11.
>
> Kevin
>

Mauro was on vacation, so pull requests were delayed. I understood from
him that he's going to process pending pull requests this week, which
should include your series.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v6 0/5] davinci: VPIF: add DT support
@ 2017-01-30  9:33         ` Hans Verkuil
  0 siblings, 0 replies; 40+ messages in thread
From: Hans Verkuil @ 2017-01-30  9:33 UTC (permalink / raw)
  To: linux-arm-kernel

On 27/01/17 18:22, Kevin Hilman wrote:
> On Fri, Dec 16, 2016 at 4:49 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>> Hans Verkuil <hverkuil@xs4all.nl> writes:
>>
>>> On 07/12/16 19:30, Kevin Hilman wrote:
>>>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>>>> This series does some fixups/cleanups and then adds the DT binding and
>>>> DT compatible string matching for DT probing.
>>>>
>>>> The controversial part from previous versions around async subdev
>>>> parsing, and specifically hard-coding the input/output routing of
>>>> subdevs, has been left out of this series.  That part can be done as a
>>>> follow-on step after agreement has been reached on the path forward.
>>>> With this version, platforms can still use the VPIF capture/display
>>>> drivers, but must provide platform_data for the subdevs and subdev
>>>> routing.
>>>>
>>>> Tested video capture to memory on da850-lcdk board using composite
>>>> input.
>>>
>>> Other than the comment for the first patch this series looks good.
>>>
>>> So once that's addressed I'll queue it up for 4.11.
>>
>> I've fixed that issue, and sent an update for just that patch in reply
>> to the original.
>>
>> Thanks for the review,
>
> Gentle ping on this series.
>
> I'm still not seeing this series yet in linux-next, so am worried it
> might not make it for v4.11.
>
> Kevin
>

Mauro was on vacation, so pull requests were delayed. I understood from
him that he's going to process pending pull requests this week, which
should include your series.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2017-01-30  9:34 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-07 18:30 [PATCH v6 0/5] davinci: VPIF: add DT support Kevin Hilman
2016-12-07 18:30 ` Kevin Hilman
2016-12-07 18:30 ` [PATCH v6 1/5] [media] davinci: VPIF: fix module loading, init errors Kevin Hilman
2016-12-07 18:30   ` Kevin Hilman
2016-12-09 10:07   ` Sakari Ailus
2016-12-09 10:07     ` Sakari Ailus
2016-12-16  9:44   ` Hans Verkuil
2016-12-16  9:44     ` Hans Verkuil
2016-12-16 23:39     ` Kevin Hilman
2016-12-16 23:39       ` Kevin Hilman
2016-12-17  0:47   ` [PATCH v6.1 " Kevin Hilman
2016-12-17  0:47     ` Kevin Hilman
2016-12-07 18:30 ` [PATCH v6 2/5] [media] davinci: vpif_capture: remove hard-coded I2C adapter id Kevin Hilman
2016-12-07 18:30   ` Kevin Hilman
2016-12-09 10:20   ` Sakari Ailus
2016-12-09 10:20     ` Sakari Ailus
2016-12-07 18:30 ` [PATCH v6 3/5] [media] davinci: vpif_capture: fix start/stop streaming locking Kevin Hilman
2016-12-07 18:30   ` Kevin Hilman
2016-12-07 18:30 ` [PATCH v6 4/5] [media] dt-bindings: add TI VPIF documentation Kevin Hilman
2016-12-07 18:30   ` Kevin Hilman
2016-12-07 18:30 ` [PATCH v6 5/5] [media] davinci: VPIF: add basic support for DT init Kevin Hilman
2016-12-07 18:30   ` Kevin Hilman
2016-12-07 20:03 ` [PATCH v6 0/5] davinci: VPIF: add DT support Javier Martinez Canillas
2016-12-07 20:03   ` Javier Martinez Canillas
2016-12-09  0:25   ` Kevin Hilman
2016-12-09  0:25     ` Kevin Hilman
2016-12-16  9:47 ` Hans Verkuil
2016-12-16  9:47   ` Hans Verkuil
2016-12-17  0:49   ` Kevin Hilman
2016-12-17  0:49     ` Kevin Hilman
2017-01-27 17:22     ` Kevin Hilman
2017-01-27 17:22       ` Kevin Hilman
2017-01-30  9:33       ` Hans Verkuil
2017-01-30  9:33         ` Hans Verkuil
2017-01-03  9:03   ` Sekhar Nori
2017-01-03  9:03     ` Sekhar Nori
2017-01-03  9:12     ` Laurent Pinchart
2017-01-03  9:12       ` Laurent Pinchart
2017-01-04 11:32       ` Sekhar Nori
2017-01-04 11:32         ` Sekhar Nori

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.