[RFC PATCH 0/6] media: davinci: VPIF: add DT support

[RFC PATCH 0/6] media: davinci: VPIF: add DT support

[Kevin Hilman]

This series attempts to add DT support to the davinci VPIF capture
driver.

I'm not sure I've completely grasped the proper use of the ports and
endpoints stuff, so this RFC is primarily to get input on whether I'm
on the right track.

The last patch is the one where all my questions are, the rest are
just prep work to ge there.

Tested on da850-lcdk and was able to do basic frame capture from the
composite input.

Series applies on v4.9-rc1

Kevin Hilman (6):
  [media] davinci: add support for DT init
  ARM: davinci: da8xx: VPIF: enable DT init
  ARM: dts: davinci: da850: add VPIF
  ARM: dts: davinci: da850-lcdk: enable VPIF capture
  [media] davinci: vpif_capture: don't lock over s_stream
  [media] davinci: vpif_capture: get subdevs from DT

 arch/arm/boot/dts/da850-lcdk.dts              |  30 ++++++
 arch/arm/boot/dts/da850.dtsi                  |  28 +++++
 arch/arm/mach-davinci/da8xx-dt.c              |  17 +++
 drivers/media/platform/davinci/vpif.c         |   9 ++
 drivers/media/platform/davinci/vpif_capture.c | 150 +++++++++++++++++++++++++-
 include/media/davinci/vpif_types.h            |   9 +-
 6 files changed, 236 insertions(+), 7 deletions(-)

-- 
2.9.3

[RFC PATCH 0/6] media: davinci: VPIF: add DT support

[Kevin Hilman]

This series attempts to add DT support to the davinci VPIF capture
driver.

I'm not sure I've completely grasped the proper use of the ports and
endpoints stuff, so this RFC is primarily to get input on whether I'm
on the right track.

The last patch is the one where all my questions are, the rest are
just prep work to ge there.

Tested on da850-lcdk and was able to do basic frame capture from the
composite input.

Series applies on v4.9-rc1

Kevin Hilman (6):
  [media] davinci: add support for DT init
  ARM: davinci: da8xx: VPIF: enable DT init
  ARM: dts: davinci: da850: add VPIF
  ARM: dts: davinci: da850-lcdk: enable VPIF capture
  [media] davinci: vpif_capture: don't lock over s_stream
  [media] davinci: vpif_capture: get subdevs from DT

 arch/arm/boot/dts/da850-lcdk.dts              |  30 ++++++
 arch/arm/boot/dts/da850.dtsi                  |  28 +++++
 arch/arm/mach-davinci/da8xx-dt.c              |  17 +++
 drivers/media/platform/davinci/vpif.c         |   9 ++
 drivers/media/platform/davinci/vpif_capture.c | 150 +++++++++++++++++++++++++-
 include/media/davinci/vpif_types.h            |   9 +-
 6 files changed, 236 insertions(+), 7 deletions(-)

-- 
2.9.3

[RFC PATCH 1/6] [media] davinci: add support for DT init

[Kevin Hilman]

Add basic support for initialization via DT.

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

diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..077e328e0281 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -464,8 +464,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,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",
 		.pm	= vpif_pm_ops,
 	},
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..79cef74e164f 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1435,6 +1435,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();
@@ -1618,8 +1623,17 @@ static int vpif_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(vpif_pm_ops, vpif_suspend, vpif_resume);
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_capture_of_match[] = {
+	{ .compatible = "ti,vpif-capture", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_capture_of_match);
+#endif
+
 static __refdata struct platform_driver vpif_driver = {
 	.driver	= {
+		.of_match_table = of_match_ptr(vpif_capture_of_match),
 		.name	= VPIF_DRIVER_NAME,
 		.pm	= &vpif_pm_ops,
 	},
-- 
2.9.3

[RFC PATCH 1/6] [media] davinci: add support for DT init

[Kevin Hilman]

Add basic support for initialization via DT.

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

diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..077e328e0281 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -464,8 +464,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,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",
 		.pm	= vpif_pm_ops,
 	},
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..79cef74e164f 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1435,6 +1435,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();
@@ -1618,8 +1623,17 @@ static int vpif_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(vpif_pm_ops, vpif_suspend, vpif_resume);
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_capture_of_match[] = {
+	{ .compatible = "ti,vpif-capture", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_capture_of_match);
+#endif
+
 static __refdata struct platform_driver vpif_driver = {
 	.driver	= {
+		.of_match_table = of_match_ptr(vpif_capture_of_match),
 		.name	= VPIF_DRIVER_NAME,
 		.pm	= &vpif_pm_ops,
 	},
-- 
2.9.3

[RFC PATCH 2/6] ARM: davinci: da8xx: VPIF: enable DT init

[Kevin Hilman]

Add basic support for DT initializaion of VPIF (capture) via DT.  Clocks
and mux still need to happen in this file until there are real clock and
pinctrl drivers, but the video nodes and subdevs can all come from DT.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 arch/arm/mach-davinci/da8xx-dt.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index c9f7e9274aa8..e1b7d72f9070 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -17,6 +17,7 @@
 #include <mach/common.h>
 #include "cp_intc.h"
 #include <mach/da8xx.h>
+#include <mach/mux.h>
 
 static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("ti,davinci-i2c", 0x01c22000, "i2c_davinci.1", NULL),
@@ -38,14 +39,30 @@ static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
 		       NULL),
 	OF_DEV_AUXDATA("ti,da830-mcasp-audio", 0x01d00000, "davinci-mcasp.0", NULL),
 	OF_DEV_AUXDATA("ti,da850-aemif", 0x68000000, "ti-aemif", NULL),
+	OF_DEV_AUXDATA("ti,vpif", 0x01e17000, "vpif", NULL),
 	{}
 };
 
 #ifdef CONFIG_ARCH_DAVINCI_DA850
 
+#if IS_ENABLED(CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE)
+static __init void da850_vpif_capture_init(void)
+{
+	int ret;
+	
+	ret = davinci_cfg_reg_list(da850_vpif_capture_pins);
+	if (ret)
+		pr_warn("da850_evm_init: VPIF capture mux setup failed: %d\n",
+			ret);
+}
+#else
+#define da850_vpif_capture_init()
+#endif
+
 static void __init da850_init_machine(void)
 {
 	of_platform_default_populate(NULL, da850_auxdata_lookup, NULL);
+	da850_vpif_capture_init();
 }
 
 static const char *const da850_boards_compat[] __initconst = {
-- 
2.9.3

[RFC PATCH 2/6] ARM: davinci: da8xx: VPIF: enable DT init

[Kevin Hilman]

Add basic support for DT initializaion of VPIF (capture) via DT.  Clocks
and mux still need to happen in this file until there are real clock and
pinctrl drivers, but the video nodes and subdevs can all come from DT.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 arch/arm/mach-davinci/da8xx-dt.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index c9f7e9274aa8..e1b7d72f9070 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -17,6 +17,7 @@
 #include <mach/common.h>
 #include "cp_intc.h"
 #include <mach/da8xx.h>
+#include <mach/mux.h>
 
 static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("ti,davinci-i2c", 0x01c22000, "i2c_davinci.1", NULL),
@@ -38,14 +39,30 @@ static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
 		       NULL),
 	OF_DEV_AUXDATA("ti,da830-mcasp-audio", 0x01d00000, "davinci-mcasp.0", NULL),
 	OF_DEV_AUXDATA("ti,da850-aemif", 0x68000000, "ti-aemif", NULL),
+	OF_DEV_AUXDATA("ti,vpif", 0x01e17000, "vpif", NULL),
 	{}
 };
 
 #ifdef CONFIG_ARCH_DAVINCI_DA850
 
+#if IS_ENABLED(CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE)
+static __init void da850_vpif_capture_init(void)
+{
+	int ret;
+	
+	ret = davinci_cfg_reg_list(da850_vpif_capture_pins);
+	if (ret)
+		pr_warn("da850_evm_init: VPIF capture mux setup failed: %d\n",
+			ret);
+}
+#else
+#define da850_vpif_capture_init()
+#endif
+
 static void __init da850_init_machine(void)
 {
 	of_platform_default_populate(NULL, da850_auxdata_lookup, NULL);
+	da850_vpif_capture_init();
 }
 
 static const char *const da850_boards_compat[] __initconst = {
-- 
2.9.3

[RFC PATCH 3/6] ARM: dts: davinci: da850: add VPIF

[Kevin Hilman]

Add VPIF and VPIF capture nodes to da850.

Note that these are separate nodes because the current media drivers
have two separate drivers for vpif and vpif_capture.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 arch/arm/boot/dts/da850.dtsi | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index f79e1b91c680..62c5b3e65071 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -399,7 +399,35 @@
 				<&edma0 0 1>;
 			dma-names = "tx", "rx";
 		};
+
+		vpif: video@0x00217000 {
+			compatible = "ti,vpif";
+			reg = <0x00217000 0x1000>;
+			status = "disabled";
+		};
+
+		vpif_capture: video-capture@0x00217000 {
+			compatible = "ti,vpif-capture";
+			reg = <0x00217000 0x1000>;
+			interrupts = <92>;
+			status = "disabled";
+
+			/* VPIF capture: input channels */
+			port {
+				vpif_ch0: endpoint@0 {
+					  reg = <0>;
+					  bus-width = <8>;
+				};
+
+				vpif_ch1: endpoint@1 {
+					  reg = <1>;
+					  bus-width = <8>;
+					  data-shift = <8>;
+				};
+			};
+		};
 	};
+
 	aemif: aemif@68000000 {
 		compatible = "ti,da850-aemif";
 		#address-cells = <2>;
-- 
2.9.3

[RFC PATCH 3/6] ARM: dts: davinci: da850: add VPIF

[Kevin Hilman]

Add VPIF and VPIF capture nodes to da850.

Note that these are separate nodes because the current media drivers
have two separate drivers for vpif and vpif_capture.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 arch/arm/boot/dts/da850.dtsi | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index f79e1b91c680..62c5b3e65071 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -399,7 +399,35 @@
 				<&edma0 0 1>;
 			dma-names = "tx", "rx";
 		};
+
+		vpif: video at 0x00217000 {
+			compatible = "ti,vpif";
+			reg = <0x00217000 0x1000>;
+			status = "disabled";
+		};
+
+		vpif_capture: video-capture at 0x00217000 {
+			compatible = "ti,vpif-capture";
+			reg = <0x00217000 0x1000>;
+			interrupts = <92>;
+			status = "disabled";
+
+			/* VPIF capture: input channels */
+			port {
+				vpif_ch0: endpoint at 0 {
+					  reg = <0>;
+					  bus-width = <8>;
+				};
+
+				vpif_ch1: endpoint at 1 {
+					  reg = <1>;
+					  bus-width = <8>;
+					  data-shift = <8>;
+				};
+			};
+		};
 	};
+
 	aemif: aemif at 68000000 {
 		compatible = "ti,da850-aemif";
 		#address-cells = <2>;
-- 
2.9.3

[RFC PATCH 4/6] ARM: dts: davinci: da850-lcdk: enable VPIF capture

[Kevin Hilman]

Enable video capture via the on-board TVP5147 decoder hooked up to ch0
one of the VPIF capture input.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 arch/arm/boot/dts/da850-lcdk.dts | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 7b8ab21fed6c..ef3c2aa1b619 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -138,6 +138,24 @@
 		reg = <0x18>;
 		status = "okay";
 	};
+
+	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>;
+			};
+		};		
+	};
 };
 
 &mcasp0 {
@@ -219,3 +237,15 @@
 		};
 	};
 };
+
+&vpif {
+	status = "okay";
+};
+
+&vpif_capture {
+	status = "okay";
+};
+
+&vpif_ch0 {
+	remote-endpoint = <&composite>;
+};
-- 
2.9.3

[RFC PATCH 4/6] ARM: dts: davinci: da850-lcdk: enable VPIF capture

[Kevin Hilman]

Enable video capture via the on-board TVP5147 decoder hooked up to ch0
one of the VPIF capture input.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 arch/arm/boot/dts/da850-lcdk.dts | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 7b8ab21fed6c..ef3c2aa1b619 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -138,6 +138,24 @@
 		reg = <0x18>;
 		status = "okay";
 	};
+
+	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>;
+			};
+		};		
+	};
 };
 
 &mcasp0 {
@@ -219,3 +237,15 @@
 		};
 	};
 };
+
+&vpif {
+	status = "okay";
+};
+
+&vpif_capture {
+	status = "okay";
+};
+
+&vpif_ch0 {
+	remote-endpoint = <&composite>;
+};
-- 
2.9.3

[RFC PATCH 5/6] [media] davinci: vpif_capture: don't lock over s_stream

[Kevin Hilman]

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

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

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 79cef74e164f..becc3e63b472 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -193,12 +193,16 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
 		}
 	}
 
+	spin_unlock_irqrestore(&common->irqlock, flags);
+
 	ret = v4l2_subdev_call(ch->sd, video, s_stream, 1);
 	if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV) {
 		vpif_dbg(1, debug, "stream on failed in subdev\n");
 		goto err;
 	}
 
+	spin_lock_irqsave(&common->irqlock, flags);
+
 	/* Call vpif_set_params function to set the parameters and addresses */
 	ret = vpif_set_video_params(vpif, ch->channel_id);
 	if (ret < 0) {
-- 
2.9.3

[RFC PATCH 5/6] [media] davinci: vpif_capture: don't lock over s_stream

[Kevin Hilman]

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

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

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 79cef74e164f..becc3e63b472 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -193,12 +193,16 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
 		}
 	}
 
+	spin_unlock_irqrestore(&common->irqlock, flags);
+
 	ret = v4l2_subdev_call(ch->sd, video, s_stream, 1);
 	if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV) {
 		vpif_dbg(1, debug, "stream on failed in subdev\n");
 		goto err;
 	}
 
+	spin_lock_irqsave(&common->irqlock, flags);
+
 	/* Call vpif_set_params function to set the parameters and addresses */
 	ret = vpif_set_video_params(vpif, ch->channel_id);
 	if (ret < 0) {
-- 
2.9.3

[RFC PATCH 6/6] [media] davinci: vpif_capture: get subdevs from DT

[Kevin Hilman]

First pass at getting subdevs from DT ports and endpoints.

The _get_pdata() function was larely inspired by (i.e. stolen from)
am437x-vpfe.c

Questions:
- Legacy board file passes subdev input & output routes via pdata
  (e.g. tvp514x svideo or composite selection.)  How is this supposed
  to be done via DT?

Not-Yet-Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 132 +++++++++++++++++++++++++-
 include/media/davinci/vpif_types.h            |   9 +-
 2 files changed, 134 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index becc3e63b472..df2af5cda37a 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -26,6 +26,8 @@
 #include <linux/slab.h>
 
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-of.h>
+#include <media/i2c/tvp514x.h> /* FIXME: how to pass the INPUT_* OUTPUT* fields? */
 
 #include "vpif.h"
 #include "vpif_capture.h"
@@ -651,6 +653,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;
@@ -658,7 +664,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;
@@ -1328,13 +1334,25 @@ static int vpif_async_bound(struct v4l2_async_notifier *notifier,
 {
 	int i;
 
+	for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
+		const struct device_node *node = vpif_obj.config->asd[i]->match.of.node;
+
+		if (node == subdev->of_node) {
+			vpif_obj.sd[i] = subdev;
+			vpif_obj.config->chan_config->inputs[i].subdev_name = subdev->of_node->full_name;
+			vpif_dbg(2, debug, "%s: setting input %d subdev_name = %s\n", __func__,
+				 i, subdev->of_node->full_name);
+			return 0;
+		}
+	}
+
 	for (i = 0; i < vpif_obj.config->subdev_count; i++)
 		if (!strcmp(vpif_obj.config->subdev_info[i].name,
 			    subdev->name)) {
 			vpif_obj.sd[i] = subdev;
 			return 0;
 		}
-
+	
 	return -EINVAL;
 }
 
@@ -1423,6 +1441,113 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
 	return vpif_probe_complete();
 }
 
+static struct vpif_capture_config *
+vpif_capture_get_pdata(struct platform_device *pdev)
+{
+	struct device_node *endpoint = NULL;
+	struct v4l2_of_endpoint bus_cfg;
+	struct vpif_capture_config *pdata;
+	struct vpif_subdev_info *sdinfo;
+	struct vpif_capture_chan_config *chan;
+	unsigned int i;
+
+	dev_dbg(&pdev->dev, "vpif_get_pdata\n");
+
+	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
+		return pdev->dev.platform_data;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+	pdata->subdev_info = devm_kzalloc(&pdev->dev,
+					  sizeof(*pdata->subdev_info) *
+					  VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
+	if (!pdata->subdev_info)
+		return NULL;
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+
+	for (i = 0; ; i++) {
+		struct device_node *rem;
+		unsigned int flags;
+		int err;
+		
+		endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
+						      endpoint);
+		if (!endpoint)
+			break;
+
+		dev_dbg(&pdev->dev, "found endpoint %s, %s\n",
+			endpoint->name, endpoint->full_name);
+
+		sdinfo = &pdata->subdev_info[i];
+		chan = &pdata->chan_config[i];
+		chan->inputs = devm_kzalloc(&pdev->dev,
+					    sizeof(*chan->inputs) *
+					    VPIF_DISPLAY_MAX_CHANNELS,
+					    GFP_KERNEL);
+		
+		/* sdinfo->name = devm_kzalloc(&pdev->dev, 16, GFP_KERNEL); */
+		/* snprintf(sdinfo->name, 16, "VPIF input %d", i); */
+		chan->input_count++;
+		chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
+		chan->inputs[i].input.std = V4L2_STD_ALL;
+		chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
+		
+		/* FIXME: need a new property? ch0:composite ch1: s-video */
+		if (i == 0)
+			chan->inputs[i].input_route = INPUT_CVBS_VI2B;
+		else
+			chan->inputs[i].input_route = INPUT_SVIDEO_VI2C_VI1C;
+		chan->inputs[i].output_route = OUTPUT_10BIT_422_EMBEDDED_SYNC;
+				
+		err = v4l2_of_parse_endpoint(endpoint, &bus_cfg);
+		if (err) {
+			dev_err(&pdev->dev, "Could not parse the endpoint\n");
+			goto done;
+		}
+		dev_dbg(&pdev->dev, "Endpoint %s, bus_width = %d\n",
+			endpoint->full_name, bus_cfg.bus.parallel.bus_width);
+		flags = bus_cfg.bus.parallel.flags;
+
+		if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
+			chan->vpif_if.hd_pol = 1;
+
+		if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
+			chan->vpif_if.vd_pol = 1;
+
+		chan->vpif_if.if_type = VPIF_IF_BT656;
+		rem = of_graph_get_remote_port_parent(endpoint);
+		if (!rem) {
+			dev_dbg(&pdev->dev, "Remote device at %s not found\n",
+				endpoint->full_name);
+			goto done;
+		}
+
+		dev_dbg(&pdev->dev, "Remote device %s, %s found\n", rem->name, rem->full_name);
+		sdinfo->name = rem->full_name;
+
+		pdata->asd[i] = devm_kzalloc(&pdev->dev,
+					     sizeof(struct v4l2_async_subdev),
+					     GFP_KERNEL);
+		if (!pdata->asd[i]) {
+			of_node_put(rem);
+			pdata = NULL;
+			goto done;
+		}
+
+		pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_OF;
+		pdata->asd[i]->match.of.node = rem;
+		of_node_put(rem);
+	}
+
+done:
+	pdata->asd_sizes[0] = i;
+	pdata->subdev_count = i;
+	pdata->card_name = "DA850/OMAP-L138 Video Capture";
+
+	return pdata;
+}
+
 /**
  * vpif_probe : This function probes the vpif capture driver
  * @pdev: platform device pointer
@@ -1439,6 +1564,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 	int res_idx = 0;
 	int i, err;
 
+	pdev->dev.platform_data = vpif_capture_get_pdata(pdev);;
 	if (!pdev->dev.platform_data) {
 		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
 		return -EINVAL;
@@ -1481,7 +1607,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 		goto vpif_unregister;
 	}
 
-	if (!vpif_obj.config->asd_sizes) {
+	if (!vpif_obj.config->asd_sizes[0]) {
 		i2c_adap = i2c_get_adapter(1);
 		for (i = 0; i < subdev_count; i++) {
 			subdevdata = &vpif_obj.config->subdev_info[i];
diff --git a/include/media/davinci/vpif_types.h b/include/media/davinci/vpif_types.h
index 3cb1704a0650..4ee3b41975db 100644
--- a/include/media/davinci/vpif_types.h
+++ b/include/media/davinci/vpif_types.h
@@ -65,14 +65,14 @@ struct vpif_display_config {
 
 struct vpif_input {
 	struct v4l2_input input;
-	const char *subdev_name;
+	char *subdev_name;
 	u32 input_route;
 	u32 output_route;
 };
 
 struct vpif_capture_chan_config {
 	struct vpif_interface vpif_if;
-	const struct vpif_input *inputs;
+	struct vpif_input *inputs;
 	int input_count;
 };
 
@@ -83,7 +83,8 @@ struct vpif_capture_config {
 	struct vpif_subdev_info *subdev_info;
 	int subdev_count;
 	const char *card_name;
-	struct v4l2_async_subdev **asd;	/* Flat array, arranged in groups */
-	int *asd_sizes;		/* 0-terminated array of asd group sizes */
+
+	struct v4l2_async_subdev *asd[VPIF_CAPTURE_MAX_CHANNELS];
+	int asd_sizes[VPIF_CAPTURE_MAX_CHANNELS];
 };
 #endif /* _VPIF_TYPES_H */
-- 
2.9.3

[RFC PATCH 6/6] [media] davinci: vpif_capture: get subdevs from DT

[Kevin Hilman]

First pass at getting subdevs from DT ports and endpoints.

The _get_pdata() function was larely inspired by (i.e. stolen from)
am437x-vpfe.c

Questions:
- Legacy board file passes subdev input & output routes via pdata
  (e.g. tvp514x svideo or composite selection.)  How is this supposed
  to be done via DT?

Not-Yet-Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 132 +++++++++++++++++++++++++-
 include/media/davinci/vpif_types.h            |   9 +-
 2 files changed, 134 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index becc3e63b472..df2af5cda37a 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -26,6 +26,8 @@
 #include <linux/slab.h>
 
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-of.h>
+#include <media/i2c/tvp514x.h> /* FIXME: how to pass the INPUT_* OUTPUT* fields? */
 
 #include "vpif.h"
 #include "vpif_capture.h"
@@ -651,6 +653,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;
@@ -658,7 +664,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;
@@ -1328,13 +1334,25 @@ static int vpif_async_bound(struct v4l2_async_notifier *notifier,
 {
 	int i;
 
+	for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
+		const struct device_node *node = vpif_obj.config->asd[i]->match.of.node;
+
+		if (node == subdev->of_node) {
+			vpif_obj.sd[i] = subdev;
+			vpif_obj.config->chan_config->inputs[i].subdev_name = subdev->of_node->full_name;
+			vpif_dbg(2, debug, "%s: setting input %d subdev_name = %s\n", __func__,
+				 i, subdev->of_node->full_name);
+			return 0;
+		}
+	}
+
 	for (i = 0; i < vpif_obj.config->subdev_count; i++)
 		if (!strcmp(vpif_obj.config->subdev_info[i].name,
 			    subdev->name)) {
 			vpif_obj.sd[i] = subdev;
 			return 0;
 		}
-
+	
 	return -EINVAL;
 }
 
@@ -1423,6 +1441,113 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
 	return vpif_probe_complete();
 }
 
+static struct vpif_capture_config *
+vpif_capture_get_pdata(struct platform_device *pdev)
+{
+	struct device_node *endpoint = NULL;
+	struct v4l2_of_endpoint bus_cfg;
+	struct vpif_capture_config *pdata;
+	struct vpif_subdev_info *sdinfo;
+	struct vpif_capture_chan_config *chan;
+	unsigned int i;
+
+	dev_dbg(&pdev->dev, "vpif_get_pdata\n");
+
+	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
+		return pdev->dev.platform_data;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+	pdata->subdev_info = devm_kzalloc(&pdev->dev,
+					  sizeof(*pdata->subdev_info) *
+					  VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
+	if (!pdata->subdev_info)
+		return NULL;
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+
+	for (i = 0; ; i++) {
+		struct device_node *rem;
+		unsigned int flags;
+		int err;
+		
+		endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
+						      endpoint);
+		if (!endpoint)
+			break;
+
+		dev_dbg(&pdev->dev, "found endpoint %s, %s\n",
+			endpoint->name, endpoint->full_name);
+
+		sdinfo = &pdata->subdev_info[i];
+		chan = &pdata->chan_config[i];
+		chan->inputs = devm_kzalloc(&pdev->dev,
+					    sizeof(*chan->inputs) *
+					    VPIF_DISPLAY_MAX_CHANNELS,
+					    GFP_KERNEL);
+		
+		/* sdinfo->name = devm_kzalloc(&pdev->dev, 16, GFP_KERNEL); */
+		/* snprintf(sdinfo->name, 16, "VPIF input %d", i); */
+		chan->input_count++;
+		chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
+		chan->inputs[i].input.std = V4L2_STD_ALL;
+		chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
+		
+		/* FIXME: need a new property? ch0:composite ch1: s-video */
+		if (i == 0)
+			chan->inputs[i].input_route = INPUT_CVBS_VI2B;
+		else
+			chan->inputs[i].input_route = INPUT_SVIDEO_VI2C_VI1C;
+		chan->inputs[i].output_route = OUTPUT_10BIT_422_EMBEDDED_SYNC;
+				
+		err = v4l2_of_parse_endpoint(endpoint, &bus_cfg);
+		if (err) {
+			dev_err(&pdev->dev, "Could not parse the endpoint\n");
+			goto done;
+		}
+		dev_dbg(&pdev->dev, "Endpoint %s, bus_width = %d\n",
+			endpoint->full_name, bus_cfg.bus.parallel.bus_width);
+		flags = bus_cfg.bus.parallel.flags;
+
+		if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
+			chan->vpif_if.hd_pol = 1;
+
+		if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
+			chan->vpif_if.vd_pol = 1;
+
+		chan->vpif_if.if_type = VPIF_IF_BT656;
+		rem = of_graph_get_remote_port_parent(endpoint);
+		if (!rem) {
+			dev_dbg(&pdev->dev, "Remote device at %s not found\n",
+				endpoint->full_name);
+			goto done;
+		}
+
+		dev_dbg(&pdev->dev, "Remote device %s, %s found\n", rem->name, rem->full_name);
+		sdinfo->name = rem->full_name;
+
+		pdata->asd[i] = devm_kzalloc(&pdev->dev,
+					     sizeof(struct v4l2_async_subdev),
+					     GFP_KERNEL);
+		if (!pdata->asd[i]) {
+			of_node_put(rem);
+			pdata = NULL;
+			goto done;
+		}
+
+		pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_OF;
+		pdata->asd[i]->match.of.node = rem;
+		of_node_put(rem);
+	}
+
+done:
+	pdata->asd_sizes[0] = i;
+	pdata->subdev_count = i;
+	pdata->card_name = "DA850/OMAP-L138 Video Capture";
+
+	return pdata;
+}
+
 /**
  * vpif_probe : This function probes the vpif capture driver
  * @pdev: platform device pointer
@@ -1439,6 +1564,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 	int res_idx = 0;
 	int i, err;
 
+	pdev->dev.platform_data = vpif_capture_get_pdata(pdev);;
 	if (!pdev->dev.platform_data) {
 		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
 		return -EINVAL;
@@ -1481,7 +1607,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 		goto vpif_unregister;
 	}
 
-	if (!vpif_obj.config->asd_sizes) {
+	if (!vpif_obj.config->asd_sizes[0]) {
 		i2c_adap = i2c_get_adapter(1);
 		for (i = 0; i < subdev_count; i++) {
 			subdevdata = &vpif_obj.config->subdev_info[i];
diff --git a/include/media/davinci/vpif_types.h b/include/media/davinci/vpif_types.h
index 3cb1704a0650..4ee3b41975db 100644
--- a/include/media/davinci/vpif_types.h
+++ b/include/media/davinci/vpif_types.h
@@ -65,14 +65,14 @@ struct vpif_display_config {
 
 struct vpif_input {
 	struct v4l2_input input;
-	const char *subdev_name;
+	char *subdev_name;
 	u32 input_route;
 	u32 output_route;
 };
 
 struct vpif_capture_chan_config {
 	struct vpif_interface vpif_if;
-	const struct vpif_input *inputs;
+	struct vpif_input *inputs;
 	int input_count;
 };
 
@@ -83,7 +83,8 @@ struct vpif_capture_config {
 	struct vpif_subdev_info *subdev_info;
 	int subdev_count;
 	const char *card_name;
-	struct v4l2_async_subdev **asd;	/* Flat array, arranged in groups */
-	int *asd_sizes;		/* 0-terminated array of asd group sizes */
+
+	struct v4l2_async_subdev *asd[VPIF_CAPTURE_MAX_CHANNELS];
+	int asd_sizes[VPIF_CAPTURE_MAX_CHANNELS];
 };
 #endif /* _VPIF_TYPES_H */
-- 
2.9.3

Re: [RFC PATCH 0/6] media: davinci: VPIF: add DT support

[Kevin Hilman]

Kevin Hilman <khilman@baylibre.com> writes:

> This series attempts to add DT support to the davinci VPIF capture
> driver.
>
> I'm not sure I've completely grasped the proper use of the ports and
> endpoints stuff, so this RFC is primarily to get input on whether I'm
> on the right track.
>
> The last patch is the one where all my questions are, the rest are
> just prep work to ge there.
>
> Tested on da850-lcdk and was able to do basic frame capture from the
> composite input.
>
> Series applies on v4.9-rc1

And FYI for anyone wanting to test, it needs a few config options
enabled[1] that are not (yet) part of davinci_all_defconfig.  I'll
update the defconfig when I'm ready to send non-RFC patches.

Kevin

[1]
CONFIG_MEDIA_SUPPORT=y
CONFIG_MEDIA_CAMERA_SUPPORT=y
CONFIG_VIDEO_DEV=y

CONFIG_VIDEO_V4L2=y
CONFIG_VIDEO_V4L2_SUBDEV_API=y

CONFIG_V4L_PLATFORM_DRIVERS=y
CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE=y

# manually select codecs
CONFIG_MEDIA_SUBDRV_AUTOSELECT=n

# da850-lcdk
CONFIG_VIDEO_TVP514X=y
CONFIG_VIDEO_ADV7343=y

[RFC PATCH 0/6] media: davinci: VPIF: add DT support

[Kevin Hilman]

Kevin Hilman <khilman@baylibre.com> writes:

> This series attempts to add DT support to the davinci VPIF capture
> driver.
>
> I'm not sure I've completely grasped the proper use of the ports and
> endpoints stuff, so this RFC is primarily to get input on whether I'm
> on the right track.
>
> The last patch is the one where all my questions are, the rest are
> just prep work to ge there.
>
> Tested on da850-lcdk and was able to do basic frame capture from the
> composite input.
>
> Series applies on v4.9-rc1

And FYI for anyone wanting to test, it needs a few config options
enabled[1] that are not (yet) part of davinci_all_defconfig.  I'll
update the defconfig when I'm ready to send non-RFC patches.

Kevin

[1]
CONFIG_MEDIA_SUPPORT=y
CONFIG_MEDIA_CAMERA_SUPPORT=y
CONFIG_VIDEO_DEV=y

CONFIG_VIDEO_V4L2=y
CONFIG_VIDEO_V4L2_SUBDEV_API=y

CONFIG_V4L_PLATFORM_DRIVERS=y
CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE=y

# manually select codecs
CONFIG_MEDIA_SUBDRV_AUTOSELECT=n

# da850-lcdk
CONFIG_VIDEO_TVP514X=y
CONFIG_VIDEO_ADV7343=y

Re: [RFC PATCH 6/6] [media] davinci: vpif_capture: get subdevs from DT

[Hans Verkuil]

On 10/26/2016 01:55 AM, Kevin Hilman wrote:
> First pass at getting subdevs from DT ports and endpoints.
> 
> The _get_pdata() function was larely inspired by (i.e. stolen from)
> am437x-vpfe.c
> 
> Questions:
> - Legacy board file passes subdev input & output routes via pdata
>   (e.g. tvp514x svideo or composite selection.)  How is this supposed
>   to be done via DT?

We have plans to model connectors as well in the device tree, but no
implementation exists yet. I think Laurent has some code in progress for this,
but I may be mistaken.

Anyway, hard-coding it like you do now is for now the only way.

	Hans

> 
> Not-Yet-Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
>  drivers/media/platform/davinci/vpif_capture.c | 132 +++++++++++++++++++++++++-
>  include/media/davinci/vpif_types.h            |   9 +-
>  2 files changed, 134 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> index becc3e63b472..df2af5cda37a 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -26,6 +26,8 @@
>  #include <linux/slab.h>
>  
>  #include <media/v4l2-ioctl.h>
> +#include <media/v4l2-of.h>
> +#include <media/i2c/tvp514x.h> /* FIXME: how to pass the INPUT_* OUTPUT* fields? */
>  
>  #include "vpif.h"
>  #include "vpif_capture.h"
> @@ -651,6 +653,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;
> @@ -658,7 +664,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;
> @@ -1328,13 +1334,25 @@ static int vpif_async_bound(struct v4l2_async_notifier *notifier,
>  {
>  	int i;
>  
> +	for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
> +		const struct device_node *node = vpif_obj.config->asd[i]->match.of.node;
> +
> +		if (node == subdev->of_node) {
> +			vpif_obj.sd[i] = subdev;
> +			vpif_obj.config->chan_config->inputs[i].subdev_name = subdev->of_node->full_name;
> +			vpif_dbg(2, debug, "%s: setting input %d subdev_name = %s\n", __func__,
> +				 i, subdev->of_node->full_name);
> +			return 0;
> +		}
> +	}
> +
>  	for (i = 0; i < vpif_obj.config->subdev_count; i++)
>  		if (!strcmp(vpif_obj.config->subdev_info[i].name,
>  			    subdev->name)) {
>  			vpif_obj.sd[i] = subdev;
>  			return 0;
>  		}
> -
> +	
>  	return -EINVAL;
>  }
>  
> @@ -1423,6 +1441,113 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
>  	return vpif_probe_complete();
>  }
>  
> +static struct vpif_capture_config *
> +vpif_capture_get_pdata(struct platform_device *pdev)
> +{
> +	struct device_node *endpoint = NULL;
> +	struct v4l2_of_endpoint bus_cfg;
> +	struct vpif_capture_config *pdata;
> +	struct vpif_subdev_info *sdinfo;
> +	struct vpif_capture_chan_config *chan;
> +	unsigned int i;
> +
> +	dev_dbg(&pdev->dev, "vpif_get_pdata\n");
> +
> +	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
> +		return pdev->dev.platform_data;
> +
> +	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return NULL;
> +	pdata->subdev_info = devm_kzalloc(&pdev->dev,
> +					  sizeof(*pdata->subdev_info) *
> +					  VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
> +	if (!pdata->subdev_info)
> +		return NULL;
> +	dev_dbg(&pdev->dev, "%s\n", __func__);
> +
> +	for (i = 0; ; i++) {
> +		struct device_node *rem;
> +		unsigned int flags;
> +		int err;
> +		
> +		endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
> +						      endpoint);
> +		if (!endpoint)
> +			break;
> +
> +		dev_dbg(&pdev->dev, "found endpoint %s, %s\n",
> +			endpoint->name, endpoint->full_name);
> +
> +		sdinfo = &pdata->subdev_info[i];
> +		chan = &pdata->chan_config[i];
> +		chan->inputs = devm_kzalloc(&pdev->dev,
> +					    sizeof(*chan->inputs) *
> +					    VPIF_DISPLAY_MAX_CHANNELS,
> +					    GFP_KERNEL);
> +		
> +		/* sdinfo->name = devm_kzalloc(&pdev->dev, 16, GFP_KERNEL); */
> +		/* snprintf(sdinfo->name, 16, "VPIF input %d", i); */
> +		chan->input_count++;
> +		chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
> +		chan->inputs[i].input.std = V4L2_STD_ALL;
> +		chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
> +		
> +		/* FIXME: need a new property? ch0:composite ch1: s-video */
> +		if (i == 0)
> +			chan->inputs[i].input_route = INPUT_CVBS_VI2B;
> +		else
> +			chan->inputs[i].input_route = INPUT_SVIDEO_VI2C_VI1C;
> +		chan->inputs[i].output_route = OUTPUT_10BIT_422_EMBEDDED_SYNC;
> +				
> +		err = v4l2_of_parse_endpoint(endpoint, &bus_cfg);
> +		if (err) {
> +			dev_err(&pdev->dev, "Could not parse the endpoint\n");
> +			goto done;
> +		}
> +		dev_dbg(&pdev->dev, "Endpoint %s, bus_width = %d\n",
> +			endpoint->full_name, bus_cfg.bus.parallel.bus_width);
> +		flags = bus_cfg.bus.parallel.flags;
> +
> +		if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> +			chan->vpif_if.hd_pol = 1;
> +
> +		if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
> +			chan->vpif_if.vd_pol = 1;
> +
> +		chan->vpif_if.if_type = VPIF_IF_BT656;
> +		rem = of_graph_get_remote_port_parent(endpoint);
> +		if (!rem) {
> +			dev_dbg(&pdev->dev, "Remote device at %s not found\n",
> +				endpoint->full_name);
> +			goto done;
> +		}
> +
> +		dev_dbg(&pdev->dev, "Remote device %s, %s found\n", rem->name, rem->full_name);
> +		sdinfo->name = rem->full_name;
> +
> +		pdata->asd[i] = devm_kzalloc(&pdev->dev,
> +					     sizeof(struct v4l2_async_subdev),
> +					     GFP_KERNEL);
> +		if (!pdata->asd[i]) {
> +			of_node_put(rem);
> +			pdata = NULL;
> +			goto done;
> +		}
> +
> +		pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_OF;
> +		pdata->asd[i]->match.of.node = rem;
> +		of_node_put(rem);
> +	}
> +
> +done:
> +	pdata->asd_sizes[0] = i;
> +	pdata->subdev_count = i;
> +	pdata->card_name = "DA850/OMAP-L138 Video Capture";
> +
> +	return pdata;
> +}
> +
>  /**
>   * vpif_probe : This function probes the vpif capture driver
>   * @pdev: platform device pointer
> @@ -1439,6 +1564,7 @@ static __init int vpif_probe(struct platform_device *pdev)
>  	int res_idx = 0;
>  	int i, err;
>  
> +	pdev->dev.platform_data = vpif_capture_get_pdata(pdev);;
>  	if (!pdev->dev.platform_data) {
>  		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
>  		return -EINVAL;
> @@ -1481,7 +1607,7 @@ static __init int vpif_probe(struct platform_device *pdev)
>  		goto vpif_unregister;
>  	}
>  
> -	if (!vpif_obj.config->asd_sizes) {
> +	if (!vpif_obj.config->asd_sizes[0]) {
>  		i2c_adap = i2c_get_adapter(1);
>  		for (i = 0; i < subdev_count; i++) {
>  			subdevdata = &vpif_obj.config->subdev_info[i];
> diff --git a/include/media/davinci/vpif_types.h b/include/media/davinci/vpif_types.h
> index 3cb1704a0650..4ee3b41975db 100644
> --- a/include/media/davinci/vpif_types.h
> +++ b/include/media/davinci/vpif_types.h
> @@ -65,14 +65,14 @@ struct vpif_display_config {
>  
>  struct vpif_input {
>  	struct v4l2_input input;
> -	const char *subdev_name;
> +	char *subdev_name;
>  	u32 input_route;
>  	u32 output_route;
>  };
>  
>  struct vpif_capture_chan_config {
>  	struct vpif_interface vpif_if;
> -	const struct vpif_input *inputs;
> +	struct vpif_input *inputs;
>  	int input_count;
>  };
>  
> @@ -83,7 +83,8 @@ struct vpif_capture_config {
>  	struct vpif_subdev_info *subdev_info;
>  	int subdev_count;
>  	const char *card_name;
> -	struct v4l2_async_subdev **asd;	/* Flat array, arranged in groups */
> -	int *asd_sizes;		/* 0-terminated array of asd group sizes */
> +
> +	struct v4l2_async_subdev *asd[VPIF_CAPTURE_MAX_CHANNELS];
> +	int asd_sizes[VPIF_CAPTURE_MAX_CHANNELS];
>  };
>  #endif /* _VPIF_TYPES_H */
> 

[RFC PATCH 6/6] [media] davinci: vpif_capture: get subdevs from DT

[Hans Verkuil]

On 10/26/2016 01:55 AM, Kevin Hilman wrote:
> First pass at getting subdevs from DT ports and endpoints.
> 
> The _get_pdata() function was larely inspired by (i.e. stolen from)
> am437x-vpfe.c
> 
> Questions:
> - Legacy board file passes subdev input & output routes via pdata
>   (e.g. tvp514x svideo or composite selection.)  How is this supposed
>   to be done via DT?

We have plans to model connectors as well in the device tree, but no
implementation exists yet. I think Laurent has some code in progress for this,
but I may be mistaken.

Anyway, hard-coding it like you do now is for now the only way.

	Hans

> 
> Not-Yet-Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
>  drivers/media/platform/davinci/vpif_capture.c | 132 +++++++++++++++++++++++++-
>  include/media/davinci/vpif_types.h            |   9 +-
>  2 files changed, 134 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> index becc3e63b472..df2af5cda37a 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -26,6 +26,8 @@
>  #include <linux/slab.h>
>  
>  #include <media/v4l2-ioctl.h>
> +#include <media/v4l2-of.h>
> +#include <media/i2c/tvp514x.h> /* FIXME: how to pass the INPUT_* OUTPUT* fields? */
>  
>  #include "vpif.h"
>  #include "vpif_capture.h"
> @@ -651,6 +653,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;
> @@ -658,7 +664,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;
> @@ -1328,13 +1334,25 @@ static int vpif_async_bound(struct v4l2_async_notifier *notifier,
>  {
>  	int i;
>  
> +	for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
> +		const struct device_node *node = vpif_obj.config->asd[i]->match.of.node;
> +
> +		if (node == subdev->of_node) {
> +			vpif_obj.sd[i] = subdev;
> +			vpif_obj.config->chan_config->inputs[i].subdev_name = subdev->of_node->full_name;
> +			vpif_dbg(2, debug, "%s: setting input %d subdev_name = %s\n", __func__,
> +				 i, subdev->of_node->full_name);
> +			return 0;
> +		}
> +	}
> +
>  	for (i = 0; i < vpif_obj.config->subdev_count; i++)
>  		if (!strcmp(vpif_obj.config->subdev_info[i].name,
>  			    subdev->name)) {
>  			vpif_obj.sd[i] = subdev;
>  			return 0;
>  		}
> -
> +	
>  	return -EINVAL;
>  }
>  
> @@ -1423,6 +1441,113 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
>  	return vpif_probe_complete();
>  }
>  
> +static struct vpif_capture_config *
> +vpif_capture_get_pdata(struct platform_device *pdev)
> +{
> +	struct device_node *endpoint = NULL;
> +	struct v4l2_of_endpoint bus_cfg;
> +	struct vpif_capture_config *pdata;
> +	struct vpif_subdev_info *sdinfo;
> +	struct vpif_capture_chan_config *chan;
> +	unsigned int i;
> +
> +	dev_dbg(&pdev->dev, "vpif_get_pdata\n");
> +
> +	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
> +		return pdev->dev.platform_data;
> +
> +	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return NULL;
> +	pdata->subdev_info = devm_kzalloc(&pdev->dev,
> +					  sizeof(*pdata->subdev_info) *
> +					  VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
> +	if (!pdata->subdev_info)
> +		return NULL;
> +	dev_dbg(&pdev->dev, "%s\n", __func__);
> +
> +	for (i = 0; ; i++) {
> +		struct device_node *rem;
> +		unsigned int flags;
> +		int err;
> +		
> +		endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
> +						      endpoint);
> +		if (!endpoint)
> +			break;
> +
> +		dev_dbg(&pdev->dev, "found endpoint %s, %s\n",
> +			endpoint->name, endpoint->full_name);
> +
> +		sdinfo = &pdata->subdev_info[i];
> +		chan = &pdata->chan_config[i];
> +		chan->inputs = devm_kzalloc(&pdev->dev,
> +					    sizeof(*chan->inputs) *
> +					    VPIF_DISPLAY_MAX_CHANNELS,
> +					    GFP_KERNEL);
> +		
> +		/* sdinfo->name = devm_kzalloc(&pdev->dev, 16, GFP_KERNEL); */
> +		/* snprintf(sdinfo->name, 16, "VPIF input %d", i); */
> +		chan->input_count++;
> +		chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
> +		chan->inputs[i].input.std = V4L2_STD_ALL;
> +		chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
> +		
> +		/* FIXME: need a new property? ch0:composite ch1: s-video */
> +		if (i == 0)
> +			chan->inputs[i].input_route = INPUT_CVBS_VI2B;
> +		else
> +			chan->inputs[i].input_route = INPUT_SVIDEO_VI2C_VI1C;
> +		chan->inputs[i].output_route = OUTPUT_10BIT_422_EMBEDDED_SYNC;
> +				
> +		err = v4l2_of_parse_endpoint(endpoint, &bus_cfg);
> +		if (err) {
> +			dev_err(&pdev->dev, "Could not parse the endpoint\n");
> +			goto done;
> +		}
> +		dev_dbg(&pdev->dev, "Endpoint %s, bus_width = %d\n",
> +			endpoint->full_name, bus_cfg.bus.parallel.bus_width);
> +		flags = bus_cfg.bus.parallel.flags;
> +
> +		if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> +			chan->vpif_if.hd_pol = 1;
> +
> +		if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
> +			chan->vpif_if.vd_pol = 1;
> +
> +		chan->vpif_if.if_type = VPIF_IF_BT656;
> +		rem = of_graph_get_remote_port_parent(endpoint);
> +		if (!rem) {
> +			dev_dbg(&pdev->dev, "Remote device at %s not found\n",
> +				endpoint->full_name);
> +			goto done;
> +		}
> +
> +		dev_dbg(&pdev->dev, "Remote device %s, %s found\n", rem->name, rem->full_name);
> +		sdinfo->name = rem->full_name;
> +
> +		pdata->asd[i] = devm_kzalloc(&pdev->dev,
> +					     sizeof(struct v4l2_async_subdev),
> +					     GFP_KERNEL);
> +		if (!pdata->asd[i]) {
> +			of_node_put(rem);
> +			pdata = NULL;
> +			goto done;
> +		}
> +
> +		pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_OF;
> +		pdata->asd[i]->match.of.node = rem;
> +		of_node_put(rem);
> +	}
> +
> +done:
> +	pdata->asd_sizes[0] = i;
> +	pdata->subdev_count = i;
> +	pdata->card_name = "DA850/OMAP-L138 Video Capture";
> +
> +	return pdata;
> +}
> +
>  /**
>   * vpif_probe : This function probes the vpif capture driver
>   * @pdev: platform device pointer
> @@ -1439,6 +1564,7 @@ static __init int vpif_probe(struct platform_device *pdev)
>  	int res_idx = 0;
>  	int i, err;
>  
> +	pdev->dev.platform_data = vpif_capture_get_pdata(pdev);;
>  	if (!pdev->dev.platform_data) {
>  		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
>  		return -EINVAL;
> @@ -1481,7 +1607,7 @@ static __init int vpif_probe(struct platform_device *pdev)
>  		goto vpif_unregister;
>  	}
>  
> -	if (!vpif_obj.config->asd_sizes) {
> +	if (!vpif_obj.config->asd_sizes[0]) {
>  		i2c_adap = i2c_get_adapter(1);
>  		for (i = 0; i < subdev_count; i++) {
>  			subdevdata = &vpif_obj.config->subdev_info[i];
> diff --git a/include/media/davinci/vpif_types.h b/include/media/davinci/vpif_types.h
> index 3cb1704a0650..4ee3b41975db 100644
> --- a/include/media/davinci/vpif_types.h
> +++ b/include/media/davinci/vpif_types.h
> @@ -65,14 +65,14 @@ struct vpif_display_config {
>  
>  struct vpif_input {
>  	struct v4l2_input input;
> -	const char *subdev_name;
> +	char *subdev_name;
>  	u32 input_route;
>  	u32 output_route;
>  };
>  
>  struct vpif_capture_chan_config {
>  	struct vpif_interface vpif_if;
> -	const struct vpif_input *inputs;
> +	struct vpif_input *inputs;
>  	int input_count;
>  };
>  
> @@ -83,7 +83,8 @@ struct vpif_capture_config {
>  	struct vpif_subdev_info *subdev_info;
>  	int subdev_count;
>  	const char *card_name;
> -	struct v4l2_async_subdev **asd;	/* Flat array, arranged in groups */
> -	int *asd_sizes;		/* 0-terminated array of asd group sizes */
> +
> +	struct v4l2_async_subdev *asd[VPIF_CAPTURE_MAX_CHANNELS];
> +	int asd_sizes[VPIF_CAPTURE_MAX_CHANNELS];
>  };
>  #endif /* _VPIF_TYPES_H */
> 

Re: [RFC PATCH 0/6] media: davinci: VPIF: add DT support

[Hans Verkuil]

Hi Kevin,

On 10/26/2016 01:55 AM, Kevin Hilman wrote:
> This series attempts to add DT support to the davinci VPIF capture
> driver.
> 
> I'm not sure I've completely grasped the proper use of the ports and
> endpoints stuff, so this RFC is primarily to get input on whether I'm
> on the right track.
> 
> The last patch is the one where all my questions are, the rest are
> just prep work to ge there.
> 
> Tested on da850-lcdk and was able to do basic frame capture from the
> composite input.
> 
> Series applies on v4.9-rc1
> 
> Kevin Hilman (6):
>   [media] davinci: add support for DT init
>   ARM: davinci: da8xx: VPIF: enable DT init
>   ARM: dts: davinci: da850: add VPIF
>   ARM: dts: davinci: da850-lcdk: enable VPIF capture
>   [media] davinci: vpif_capture: don't lock over s_stream
>   [media] davinci: vpif_capture: get subdevs from DT

Looks good, but wouldn't it be better to do the dts changes last when all the
supporting code is in?

Regards,

	Hans

> 
>  arch/arm/boot/dts/da850-lcdk.dts              |  30 ++++++
>  arch/arm/boot/dts/da850.dtsi                  |  28 +++++
>  arch/arm/mach-davinci/da8xx-dt.c              |  17 +++
>  drivers/media/platform/davinci/vpif.c         |   9 ++
>  drivers/media/platform/davinci/vpif_capture.c | 150 +++++++++++++++++++++++++-
>  include/media/davinci/vpif_types.h            |   9 +-
>  6 files changed, 236 insertions(+), 7 deletions(-)
> 

[RFC PATCH 0/6] media: davinci: VPIF: add DT support

[Hans Verkuil]

Hi Kevin,

On 10/26/2016 01:55 AM, Kevin Hilman wrote:
> This series attempts to add DT support to the davinci VPIF capture
> driver.
> 
> I'm not sure I've completely grasped the proper use of the ports and
> endpoints stuff, so this RFC is primarily to get input on whether I'm
> on the right track.
> 
> The last patch is the one where all my questions are, the rest are
> just prep work to ge there.
> 
> Tested on da850-lcdk and was able to do basic frame capture from the
> composite input.
> 
> Series applies on v4.9-rc1
> 
> Kevin Hilman (6):
>   [media] davinci: add support for DT init
>   ARM: davinci: da8xx: VPIF: enable DT init
>   ARM: dts: davinci: da850: add VPIF
>   ARM: dts: davinci: da850-lcdk: enable VPIF capture
>   [media] davinci: vpif_capture: don't lock over s_stream
>   [media] davinci: vpif_capture: get subdevs from DT

Looks good, but wouldn't it be better to do the dts changes last when all the
supporting code is in?

Regards,

	Hans

> 
>  arch/arm/boot/dts/da850-lcdk.dts              |  30 ++++++
>  arch/arm/boot/dts/da850.dtsi                  |  28 +++++
>  arch/arm/mach-davinci/da8xx-dt.c              |  17 +++
>  drivers/media/platform/davinci/vpif.c         |   9 ++
>  drivers/media/platform/davinci/vpif_capture.c | 150 +++++++++++++++++++++++++-
>  include/media/davinci/vpif_types.h            |   9 +-
>  6 files changed, 236 insertions(+), 7 deletions(-)
> 

Re: [RFC PATCH 6/6] [media] davinci: vpif_capture: get subdevs from DT

[Javier Martinez Canillas]

Hello,

On Fri, Nov 11, 2016 at 12:36 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 10/26/2016 01:55 AM, Kevin Hilman wrote:
>> First pass at getting subdevs from DT ports and endpoints.
>>
>> The _get_pdata() function was larely inspired by (i.e. stolen from)
>> am437x-vpfe.c
>>
>> Questions:
>> - Legacy board file passes subdev input & output routes via pdata
>>   (e.g. tvp514x svideo or composite selection.)  How is this supposed
>>   to be done via DT?
>
> We have plans to model connectors as well in the device tree, but no
> implementation exists yet. I think Laurent has some code in progress for this,
> but I may be mistaken.
>

I posted a RFC series [0] some time ago, that proposed a DT binding
for input connectors [1] using OF graphs.

I never re-spin the series because Laurent had some comments on the DT
bindings and I was waiting for a response on to my latest email [2].
So if you can comment on this and see if the DT bindings fits your,
would be very useful.

> Anyway, hard-coding it like you do now is for now the only way.
>
>         Hans
>
>>

[0]: https://www.mail-archive.com/linux-media@vger.kernel.org/msg96393.html
[1]: http://www.spinics.net/lists/linux-media/msg99421.html
[2]: http://www.spinics.net/lists/linux-media/msg99987.html

Best regards,
Javier

[RFC PATCH 6/6] [media] davinci: vpif_capture: get subdevs from DT

[Javier Martinez Canillas]

Hello,

On Fri, Nov 11, 2016 at 12:36 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 10/26/2016 01:55 AM, Kevin Hilman wrote:
>> First pass at getting subdevs from DT ports and endpoints.
>>
>> The _get_pdata() function was larely inspired by (i.e. stolen from)
>> am437x-vpfe.c
>>
>> Questions:
>> - Legacy board file passes subdev input & output routes via pdata
>>   (e.g. tvp514x svideo or composite selection.)  How is this supposed
>>   to be done via DT?
>
> We have plans to model connectors as well in the device tree, but no
> implementation exists yet. I think Laurent has some code in progress for this,
> but I may be mistaken.
>

I posted a RFC series [0] some time ago, that proposed a DT binding
for input connectors [1] using OF graphs.

I never re-spin the series because Laurent had some comments on the DT
bindings and I was waiting for a response on to my latest email [2].
So if you can comment on this and see if the DT bindings fits your,
would be very useful.

> Anyway, hard-coding it like you do now is for now the only way.
>
>         Hans
>
>>

[0]: https://www.mail-archive.com/linux-media at vger.kernel.org/msg96393.html
[1]: http://www.spinics.net/lists/linux-media/msg99421.html
[2]: http://www.spinics.net/lists/linux-media/msg99987.html

Best regards,
Javier

Re: [RFC PATCH 6/6] [media] davinci: vpif_capture: get subdevs from DT

[Javier Martinez Canillas]

On Fri, Nov 11, 2016 at 12:50 PM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:

[snip]

> So if you can comment on this and see if the DT bindings fits your,
> would be very useful.
>

Sorry, I wanted to write "if the DT bindings fits your needs, it would
be very useful".

Best regards,
Javier

[RFC PATCH 6/6] [media] davinci: vpif_capture: get subdevs from DT

[Javier Martinez Canillas]

On Fri, Nov 11, 2016 at 12:50 PM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:

[snip]

> So if you can comment on this and see if the DT bindings fits your,
> would be very useful.
>

Sorry, I wanted to write "if the DT bindings fits your needs, it would
be very useful".

Best regards,
Javier

Re: [RFC PATCH 0/6] media: davinci: VPIF: add DT support

[Kevin Hilman]

Hans Verkuil <hverkuil@xs4all.nl> writes:

> Hi Kevin,
>
> On 10/26/2016 01:55 AM, Kevin Hilman wrote:
>> This series attempts to add DT support to the davinci VPIF capture
>> driver.
>> 
>> I'm not sure I've completely grasped the proper use of the ports and
>> endpoints stuff, so this RFC is primarily to get input on whether I'm
>> on the right track.
>> 
>> The last patch is the one where all my questions are, the rest are
>> just prep work to ge there.
>> 
>> Tested on da850-lcdk and was able to do basic frame capture from the
>> composite input.
>> 
>> Series applies on v4.9-rc1
>> 
>> Kevin Hilman (6):
>>   [media] davinci: add support for DT init
>>   ARM: davinci: da8xx: VPIF: enable DT init
>>   ARM: dts: davinci: da850: add VPIF
>>   ARM: dts: davinci: da850-lcdk: enable VPIF capture
>>   [media] davinci: vpif_capture: don't lock over s_stream
>>   [media] davinci: vpif_capture: get subdevs from DT
>
> Looks good, but wouldn't it be better to do the dts changes last when all the
> supporting code is in?

I guess it doesn't really matter in this case, because the DT nodes will
be nops until the driver changes are in.

Either way, next week I'll repost a non-RFC version and separate out the
arch and DT patches, since those will go through Sekhar's davinci tree,
and then via arm-soc.

Thanks for the review,

Kevin

[RFC PATCH 0/6] media: davinci: VPIF: add DT support

[Kevin Hilman]

Hans Verkuil <hverkuil@xs4all.nl> writes:

> Hi Kevin,
>
> On 10/26/2016 01:55 AM, Kevin Hilman wrote:
>> This series attempts to add DT support to the davinci VPIF capture
>> driver.
>> 
>> I'm not sure I've completely grasped the proper use of the ports and
>> endpoints stuff, so this RFC is primarily to get input on whether I'm
>> on the right track.
>> 
>> The last patch is the one where all my questions are, the rest are
>> just prep work to ge there.
>> 
>> Tested on da850-lcdk and was able to do basic frame capture from the
>> composite input.
>> 
>> Series applies on v4.9-rc1
>> 
>> Kevin Hilman (6):
>>   [media] davinci: add support for DT init
>>   ARM: davinci: da8xx: VPIF: enable DT init
>>   ARM: dts: davinci: da850: add VPIF
>>   ARM: dts: davinci: da850-lcdk: enable VPIF capture
>>   [media] davinci: vpif_capture: don't lock over s_stream
>>   [media] davinci: vpif_capture: get subdevs from DT
>
> Looks good, but wouldn't it be better to do the dts changes last when all the
> supporting code is in?

I guess it doesn't really matter in this case, because the DT nodes will
be nops until the driver changes are in.

Either way, next week I'll repost a non-RFC version and separate out the
arch and DT patches, since those will go through Sekhar's davinci tree,
and then via arm-soc.

Thanks for the review,

Kevin