soc_camera rcar_vin support for device-tree binding

soc_camera rcar_vin support for device-tree binding

[Ben Dooks]

This is a series of patches to get towards the point the renesas
rcar_vin driver can be bound via device-tree. Patches 1 and 2 add
the device tree nodes, patch 3 is one which has been in my tree
for a while and the last two modify the rcar_vin driver.

[PATCH 1/5] r8a7790.dtsi: add vin[0-3] nodes

[Ben Dooks]

Add nodes for the four video input channels on the R8A7790.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 arch/arm/boot/dts/r8a7790.dtsi | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index a1e7c39..4c3eafb 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -395,6 +395,38 @@
 		status = "disabled";
 	};
 
+	vin0: vin@0xe6ef0000 {
+		compatible = "renesas,vin-r8a7790";
+		clocks = <&mstp8_clks R8A7790_CLK_VIN0>;
+		reg = <0 0xe6ef0000 0 0x1000>;
+		interrupts = <0 188 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	vin1: vin@0xe6ef1000 {
+		compatible = "renesas,vin-r8a7790";
+		clocks = <&mstp8_clks R8A7790_CLK_VIN1>;
+		reg = <0 0xe6ef1000 0 0x1000>;
+		interrupts = <0 189 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	vin2: vin@0xe6ef2000 {
+		compatible = "renesas,vin-r8a7790";
+		clocks = <&mstp8_clks R8A7790_CLK_VIN2>;
+		reg = <0 0xe6ef2000 0 0x1000>;
+		interrupts = <0 190 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	vin3: vin@0xe6ef3000 {
+		compatible = "renesas,vin-r8a7790";
+		clocks = <&mstp8_clks R8A7790_CLK_VIN3>;
+		reg = <0 0xe6ef3000 0 0x1000>;
+		interrupts = <0 191 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
 	clocks {
 		#address-cells = <2>;
 		#size-cells = <2>;
-- 
1.9.0

[PATCH 2/5] ARM: lager: add vin1 node

[Ben Dooks]

Add device-tree for vin1 (composite video in) on the
lager board.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 arch/arm/boot/dts/r8a7790-lager.dts | 38 +++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index a087421..7528cfc 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -158,6 +158,11 @@
 		renesas,groups = "i2c2";
 		renesas,function = "i2c2";
 	};
+
+	vin1_pins: vin {
+		renesas,groups = "vin1_data8", "vin1_clk";
+		renesas,function = "vin1";
+	};
 };
 
 &mmcif1 {
@@ -239,8 +244,41 @@
 	status = "ok";
 	pinctrl-0 = <&i2c2_pins>;
 	pinctrl-names = "default";
+
+	adv7180: adv7180@0x20 {
+		compatible = "adi,adv7180";
+		reg = <0x20>;
+		remote = <&vin1>;
+
+		port {
+			adv7180_1: endpoint {
+				bus-width = <8>;
+				remote-endpoint = <&vin1ep0>;
+			};
+		};
+	};
+
 };
 
 &i2c3	{
 	status = "ok";
 };
+
+/* composite video input */
+&vin1 {
+	pinctrl-0 = <&vin1_pins>;
+	pinctrl-names = "default";
+
+	status = "ok";
+
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		vin1ep0: endpoint {
+			remote-endpoint = <&adv7180_1>;
+			bus-width = <8>;
+		};
+	};
+};
+
-- 
1.9.0

[PATCH 3/5] media: soc_camera: rcar_vin: Add support for 10-bit YUV cameras

[Ben Dooks]

From: Phil Edworthy <phil.edworthy@renesas.com>

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/media/platform/soc_camera/rcar_vin.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index 3b1c05a..702dc47 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -68,6 +68,8 @@
 #define VNMC_YCAL		(1 << 19)
 #define VNMC_INF_YUV8_BT656	(0 << 16)
 #define VNMC_INF_YUV8_BT601	(1 << 16)
+#define VNMC_INF_YUV10_BT656	(2 << 16)
+#define VNMC_INF_YUV10_BT601	(3 << 16)
 #define VNMC_INF_YUV16		(5 << 16)
 #define VNMC_VUP		(1 << 10)
 #define VNMC_IM_ODD		(0 << 3)
@@ -275,6 +277,12 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
 		/* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
 		vnmc |= priv->pdata->flags & RCAR_VIN_BT656 ?
 			VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
+		break;
+	case V4L2_MBUS_FMT_YUYV10_2X10:
+		/* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
+		vnmc |= priv->pdata->flags & RCAR_VIN_BT656 ?
+			VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
+		break;
 	default:
 		break;
 	}
@@ -1003,6 +1011,7 @@ static int rcar_vin_get_formats(struct soc_camera_device *icd, unsigned int idx,
 	switch (code) {
 	case V4L2_MBUS_FMT_YUYV8_1X16:
 	case V4L2_MBUS_FMT_YUYV8_2X8:
+	case V4L2_MBUS_FMT_YUYV10_2X10:
 		if (cam->extra_fmt)
 			break;
 
-- 
1.9.0

[PATCH 4/5] rcar_vin: copy flags from pdata

[Ben Dooks]

The platform data is a single word, so simply copy
it into the device's private data structure than
keeping a copy of the pointer.

This will make changing to device-tree binding
easier as it is one allocation instead of two.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/media/platform/soc_camera/rcar_vin.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index 702dc47..47516df 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -126,13 +126,13 @@ struct rcar_vin_priv {
 	int				sequence;
 	/* State of the VIN module in capturing mode */
 	enum rcar_vin_state		state;
-	struct rcar_vin_platform_data	*pdata;
 	struct soc_camera_host		ici;
 	struct list_head		capture;
 #define MAX_BUFFER_NUM			3
 	struct vb2_buffer		*queue_buf[MAX_BUFFER_NUM];
 	struct vb2_alloc_ctx		*alloc_ctx;
 	enum v4l2_field			field;
+	unsigned int			pdata_flags;
 	unsigned int			vb_count;
 	unsigned int			nr_hw_slots;
 	bool				request_to_stop;
@@ -275,12 +275,12 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
 		break;
 	case V4L2_MBUS_FMT_YUYV8_2X8:
 		/* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
-		vnmc |= priv->pdata->flags & RCAR_VIN_BT656 ?
+		vnmc |= priv->pdata_flags & RCAR_VIN_BT656 ?
 			VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
 		break;
 	case V4L2_MBUS_FMT_YUYV10_2X10:
 		/* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
-		vnmc |= priv->pdata->flags & RCAR_VIN_BT656 ?
+		vnmc |= priv->pdata_flags & RCAR_VIN_BT656 ?
 			VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
 		break;
 	default:
@@ -799,7 +799,7 @@ static int rcar_vin_set_bus_param(struct soc_camera_device *icd)
 	/* Make choises, based on platform preferences */
 	if ((common_flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) &&
 	    (common_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)) {
-		if (priv->pdata->flags & RCAR_VIN_HSYNC_ACTIVE_LOW)
+		if (priv->pdata_flags & RCAR_VIN_HSYNC_ACTIVE_LOW)
 			common_flags &= ~V4L2_MBUS_HSYNC_ACTIVE_HIGH;
 		else
 			common_flags &= ~V4L2_MBUS_HSYNC_ACTIVE_LOW;
@@ -807,7 +807,7 @@ static int rcar_vin_set_bus_param(struct soc_camera_device *icd)
 
 	if ((common_flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) &&
 	    (common_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)) {
-		if (priv->pdata->flags & RCAR_VIN_VSYNC_ACTIVE_LOW)
+		if (priv->pdata_flags & RCAR_VIN_VSYNC_ACTIVE_LOW)
 			common_flags &= ~V4L2_MBUS_VSYNC_ACTIVE_HIGH;
 		else
 			common_flags &= ~V4L2_MBUS_VSYNC_ACTIVE_LOW;
@@ -1447,7 +1447,7 @@ static int rcar_vin_probe(struct platform_device *pdev)
 	priv->ici.drv_name = dev_name(&pdev->dev);
 	priv->ici.ops = &rcar_vin_host_ops;
 
-	priv->pdata = pdata;
+	priv->pdata_flags = pdata->flags;
 	priv->chip = pdev->id_entry->driver_data;
 	spin_lock_init(&priv->lock);
 	INIT_LIST_HEAD(&priv->capture);
-- 
1.9.0

[PATCH 5/5] rcar_vin: add devicetree support

[Ben Dooks]

Add support for devicetree probe for the rcar-vin
driver.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 .../devicetree/bindings/media/rcar_vin.txt         | 79 ++++++++++++++++++++++
 drivers/media/platform/soc_camera/rcar_vin.c       | 67 ++++++++++++++++--
 2 files changed, 140 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/rcar_vin.txt

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
new file mode 100644
index 0000000..105b8de
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -0,0 +1,79 @@
+Renesas RCar Video Input driver (rcar_vin)
+------------------------------------------
+
+The rcar_vin device provides video input capabilities for the Renesas R-Car
+family of devices. The current blocks are always slaves and suppot one input
+channel which can be either RGB, YUYV or BT656.
+
+ - compatible: Must be one of the following
+   - "renesas,vin-r8a7791" for the R8A7791 device
+   - "renesas,vin-r8a7790" for the R8A7790 device
+   - "renesas,vin-r8a7779" for the R8A7779 device
+   - "renesas,vin-r8a7778" for the R8A7778 device
+ - reg: the register base and size for the device registers
+ - interrupts: the interrupt for the device
+ - clocks: Reference to the parent clock
+
+The per-board settings:
+ - port sub-node describing a single endpoint connected to the vin
+   as described in video-interfaces.txt[1]. Only the first one will
+   be considered as each vin interface has one input port.
+
+   These settings are used to work out video input format and widths
+   into the system.
+
+
+Device node example
+-------------------
+
+        vin0: vin@0xe6ef0000 {
+                compatible = "renesas,vin-r8a7790";
+                clocks = <&mstp8_clks R8A7790_CLK_VIN0>;
+                reg = <0 0xe6ef0000 0 0x1000>;
+                interrupts = <0 188 IRQ_TYPE_LEVEL_HIGH>;
+                status = "disabled";
+        };
+
+Board setup example (vin1 composite video input)
+------------------------------------------------
+
+&i2c2   {
+        status = "ok";
+        pinctrl-0 = <&i2c2_pins>;
+        pinctrl-names = "default";
+
+        adv7180: adv7180@0x20 {
+                compatible = "adi,adv7180";
+                reg = <0x20>;
+                remote = <&vin1>;
+
+                port {
+                        adv7180_1: endpoint {
+                                bus-width = <8>;
+                                remote-endpoint = <&vin1ep0>;
+                        };
+                };
+        };
+};
+
+/* composite video input */
+&vin1 {
+        pinctrl-0 = <&vin1_pins>;
+        pinctrl-names = "default";
+
+        status = "ok";
+
+        port {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                vin1ep0: endpoint {
+                        remote-endpoint = <&adv7180_1>;
+                        bus-width = <8>;
+                };
+        };
+};
+
+
+
+[1] video-interfaces.txt common video media interface
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index 47516df..73c56c7 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -24,6 +24,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
 #include <linux/videodev2.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <media/soc_camera.h>
 #include <media/soc_mediabus.h>
@@ -32,6 +34,7 @@
 #include <media/v4l2-device.h>
 #include <media/v4l2-mediabus.h>
 #include <media/v4l2-subdev.h>
+#include <media/v4l2-of.h>
 #include <media/videobuf2-dma-contig.h>
 
 #include "soc_scale_crop.h"
@@ -1392,6 +1395,17 @@ static struct soc_camera_host_ops rcar_vin_host_ops = {
 	.init_videobuf2	= rcar_vin_init_videobuf2,
 };
 
+#ifdef CONFIG_OF
+static struct of_device_id rcar_vin_of_table[] = {
+	{ .compatible = "renesas,vin-r8a7791", .data = (void *)RCAR_GEN2 },
+	{ .compatible = "renesas,vin-r8a7790", .data = (void *)RCAR_GEN2 },
+	{ .compatible = "renesas,vin-r8a7779", .data = (void *)RCAR_H1 },
+	{ .compatible = "renesas,vin-r8a7778", .data = (void *)RCAR_M1 },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, rcar_vin_of_table);
+#endif
+
 static struct platform_device_id rcar_vin_id_table[] = {
 	{ "r8a7791-vin",  RCAR_GEN2 },
 	{ "r8a7790-vin",  RCAR_GEN2 },
@@ -1404,15 +1418,50 @@ MODULE_DEVICE_TABLE(platform, rcar_vin_id_table);
 
 static int rcar_vin_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *match = NULL;
 	struct rcar_vin_priv *priv;
 	struct resource *mem;
 	struct rcar_vin_platform_data *pdata;
+	unsigned int pdata_flags;
 	int irq, ret;
 
-	pdata = pdev->dev.platform_data;
-	if (!pdata || !pdata->flags) {
-		dev_err(&pdev->dev, "platform data not set\n");
-		return -EINVAL;
+	if (pdev->dev.of_node) {
+		struct v4l2_of_endpoint ep;
+		struct device_node *np;
+
+		match = of_match_device(of_match_ptr(rcar_vin_of_table),
+					&pdev->dev);
+
+		np = v4l2_of_get_next_endpoint(pdev->dev.of_node, NULL);
+		if (!np) {
+			dev_err(&pdev->dev, "could not find endpoint\n");
+			return -EINVAL;
+		}
+
+		ret = v4l2_of_parse_endpoint(np, &ep);
+		if (ret) {
+			dev_err(&pdev->dev, "could not parse endpoint\n");
+			return ret;
+		}
+
+		if (ep.bus_type = V4L2_MBUS_BT656)
+			pdata_flags = RCAR_VIN_BT656;
+		else {
+			pdata_flags = 0;
+			if (ep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)
+				pdata_flags |= RCAR_VIN_HSYNC_ACTIVE_LOW;
+			if (ep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)
+				pdata_flags |= RCAR_VIN_VSYNC_ACTIVE_LOW;
+		}
+
+		dev_dbg(&pdev->dev, "pdata_flags = %08x\n", pdata_flags);
+	} else {
+		pdata = pdev->dev.platform_data;
+		if (!pdata || !pdata->flags) {
+			dev_err(&pdev->dev, "platform data not set\n");
+			return -EINVAL;
+		}
+		pdata_flags = pdata->flags;
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1447,8 +1496,13 @@ static int rcar_vin_probe(struct platform_device *pdev)
 	priv->ici.drv_name = dev_name(&pdev->dev);
 	priv->ici.ops = &rcar_vin_host_ops;
 
-	priv->pdata_flags = pdata->flags;
-	priv->chip = pdev->id_entry->driver_data;
+	priv->pdata_flags = pdata_flags;
+	if (!match)
+		priv->chip = pdev->id_entry->driver_data;
+	else
+		priv->chip = (enum chip_id)match->data;
+
+
 	spin_lock_init(&priv->lock);
 	INIT_LIST_HEAD(&priv->capture);
 
@@ -1489,6 +1543,7 @@ static struct platform_driver rcar_vin_driver = {
 	.driver		= {
 		.name		= DRV_NAME,
 		.owner		= THIS_MODULE,
+		.of_match_table	= of_match_ptr(rcar_vin_of_table),
 	},
 	.id_table	= rcar_vin_id_table,
 };
-- 
1.9.0

Re: [PATCH 5/5] rcar_vin: add devicetree support

[Geert Uytterhoeven]

On Fri, Mar 7, 2014 at 2:01 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt

> +family of devices. The current blocks are always slaves and suppot one input

support

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/5] r8a7790.dtsi: add vin[0-3] nodes

[Sergei Shtylyov]

Hello.

On 03/07/2014 04:01 PM, Ben Dooks wrote:

> Add nodes for the four video input channels on the R8A7790.

> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>

    This patch should have been preceded by the VIN driver patch and bindings 
description, don't you think?

> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index a1e7c39..4c3eafb 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -395,6 +395,38 @@
>   		status = "disabled";
>   	};
>
> +	vin0: vin@0xe6ef0000 {

    ePAPR standard [1] tells us that:

The name of a node should be somewhat generic, reflecting the function of the 
device and not its precise programming model.

    So, I would suggest something like "video". And remove "0x" from the 
address part of the node name please.

WBR, Sergei

Re: [PATCH 2/5] ARM: lager: add vin1 node

[Sergei Shtylyov]

Hello.

On 03/07/2014 04:01 PM, Ben Dooks wrote:

> Add device-tree for vin1 (composite video in) on the
> lager board.

> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>

    This patch should have been preceded by the VIN driver patch and bindings 
description, don't you think?

> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index a087421..7528cfc 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
[...]
> @@ -239,8 +244,41 @@
>   	status = "ok";
>   	pinctrl-0 = <&i2c2_pins>;
>   	pinctrl-names = "default";
> +
> +	adv7180: adv7180@0x20 {

    ePAPR standard [1] tells us that:

"The name of a node should be somewhat generic, reflecting the function of the 
device and not its precise programming model."

    So, I would suggest something like "video-decoder" instead. And remove 
"0x" from the address part of the node name please.

[1] http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf

WBR, Sergei

Re: [PATCH 1/5] r8a7790.dtsi: add vin[0-3] nodes

[Ben Dooks]

On 07/03/14 16:45, Sergei Shtylyov wrote:
> Hello.
>
> On 03/07/2014 04:01 PM, Ben Dooks wrote:
>
>> Add nodes for the four video input channels on the R8A7790.
>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>
>     This patch should have been preceded by the VIN driver patch and
> bindings description, don't you think?

Given this is a pretty standard device and video input binding
as already described in the relevant documentation, didn't really
think it was necessary.

>> diff --git a/arch/arm/boot/dts/r8a7790.dtsi
>> b/arch/arm/boot/dts/r8a7790.dtsi
>> index a1e7c39..4c3eafb 100644
>> --- a/arch/arm/boot/dts/r8a7790.dtsi
>> +++ b/arch/arm/boot/dts/r8a7790.dtsi
>> @@ -395,6 +395,38 @@
>>           status = "disabled";
>>       };
>>
>> +    vin0: vin@0xe6ef0000 {
>
>     ePAPR standard [1] tells us that:
>
> The name of a node should be somewhat generic, reflecting the function
> of the device and not its precise programming model.
>
>     So, I would suggest something like "video". And remove "0x" from the
> address part of the node name please.

vin is a reasonable contraction of video-input.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH 2/5] ARM: lager: add vin1 node

[Ben Dooks]

On 07/03/14 16:50, Sergei Shtylyov wrote:
> Hello.
>
> On 03/07/2014 04:01 PM, Ben Dooks wrote:
>
>> Add device-tree for vin1 (composite video in) on the
>> lager board.
>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>
>     This patch should have been preceded by the VIN driver patch and
> bindings description, don't you think?
>
>> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts
>> b/arch/arm/boot/dts/r8a7790-lager.dts
>> index a087421..7528cfc 100644
>> --- a/arch/arm/boot/dts/r8a7790-lager.dts
>> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> [...]
>> @@ -239,8 +244,41 @@
>>       status = "ok";
>>       pinctrl-0 = <&i2c2_pins>;
>>       pinctrl-names = "default";
>> +
>> +    adv7180: adv7180@0x20 {
>
>     ePAPR standard [1] tells us that:
>
> "The name of a node should be somewhat generic, reflecting the function
> of the device and not its precise programming model."
>
>     So, I would suggest something like "video-decoder" instead. And
> remove "0x" from the address part of the node name please.

Personally I'm not fussed about names in the tree.

I will look at changing it to something like vin1_decoder


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH 5/5] rcar_vin: add devicetree support

[Sergei Shtylyov]

Hello.

On 03/07/2014 04:01 PM, Ben Dooks wrote:

> Add support for devicetree probe for the rcar-vin
> driver.

> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
>   .../devicetree/bindings/media/rcar_vin.txt         | 79 ++++++++++++++++++++++
>   drivers/media/platform/soc_camera/rcar_vin.c       | 67 ++++++++++++++++--
>   2 files changed, 140 insertions(+), 6 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/media/rcar_vin.txt

> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> new file mode 100644
> index 0000000..105b8de
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -0,0 +1,79 @@
> +Renesas RCar Video Input driver (rcar_vin)
> +------------------------------------------
> +
> +The rcar_vin device provides video input capabilities for the Renesas R-Car
> +family of devices. The current blocks are always slaves and suppot one input

    Support.

> +channel which can be either RGB, YUYV or BT656.
> +
> + - compatible: Must be one of the following
> +   - "renesas,vin-r8a7791" for the R8A7791 device
> +   - "renesas,vin-r8a7790" for the R8A7790 device
> +   - "renesas,vin-r8a7779" for the R8A7779 device
> +   - "renesas,vin-r8a7778" for the R8A7778 device
> + - reg: the register base and size for the device registers
> + - interrupts: the interrupt for the device
> + - clocks: Reference to the parent clock
> +
> +The per-board settings:
> + - port sub-node describing a single endpoint connected to the vin

    s/vin/VIN/.

> +   as described in video-interfaces.txt[1]. Only the first one will
> +   be considered as each vin interface has one input port.
[...]
> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
> index 47516df..73c56c7 100644
> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -1404,15 +1418,50 @@ MODULE_DEVICE_TABLE(platform, rcar_vin_id_table);
>
>   static int rcar_vin_probe(struct platform_device *pdev)
>   {
> +	const struct of_device_id *match = NULL;
>   	struct rcar_vin_priv *priv;
>   	struct resource *mem;
>   	struct rcar_vin_platform_data *pdata;
> +	unsigned int pdata_flags;
>   	int irq, ret;
>
> -	pdata = pdev->dev.platform_data;
> -	if (!pdata || !pdata->flags) {
> -		dev_err(&pdev->dev, "platform data not set\n");
> -		return -EINVAL;
> +	if (pdev->dev.of_node) {
> +		struct v4l2_of_endpoint ep;
> +		struct device_node *np;
> +
> +		match = of_match_device(of_match_ptr(rcar_vin_of_table),
> +					&pdev->dev);
> +
> +		np = v4l2_of_get_next_endpoint(pdev->dev.of_node, NULL);
> +		if (!np) {
> +			dev_err(&pdev->dev, "could not find endpoint\n");
> +			return -EINVAL;
> +		}
> +
> +		ret = v4l2_of_parse_endpoint(np, &ep);
> +		if (ret) {
> +			dev_err(&pdev->dev, "could not parse endpoint\n");
> +			return ret;
> +		}
> +
> +		if (ep.bus_type = V4L2_MBUS_BT656)
> +			pdata_flags = RCAR_VIN_BT656;
> +		else {

    Both arms of *if* should have {} -- see Documentation/CodingStyle.

> +			pdata_flags = 0;
> +			if (ep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)
> +				pdata_flags |= RCAR_VIN_HSYNC_ACTIVE_LOW;
> +			if (ep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)
> +				pdata_flags |= RCAR_VIN_VSYNC_ACTIVE_LOW;
> +		}
> +
> +		dev_dbg(&pdev->dev, "pdata_flags = %08x\n", pdata_flags);
> +	} else {
> +		pdata = pdev->dev.platform_data;

    Use dev_get_platdata(&pdev->dev), please. Lot of drivers have been 
converted to use it instead of the direct access. Haven't gotten to this one 
it seems.

> @@ -1447,8 +1496,13 @@ static int rcar_vin_probe(struct platform_device *pdev)
>   	priv->ici.drv_name = dev_name(&pdev->dev);
>   	priv->ici.ops = &rcar_vin_host_ops;
>
> -	priv->pdata_flags = pdata->flags;
> -	priv->chip = pdev->id_entry->driver_data;
> +	priv->pdata_flags = pdata_flags;
> +	if (!match)
> +		priv->chip = pdev->id_entry->driver_data;
> +	else
> +		priv->chip = (enum chip_id)match->data;
> +
> +

    Too many empty lines here, I think.

WBR, Sergei

Re: [PATCH 2/5] ARM: lager: add vin1 node

[Sergei Shtylyov]

On 03/07/2014 07:06 PM, Ben Dooks wrote:

>>> Add device-tree for vin1 (composite video in) on the
>>> lager board.

>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>

>>     This patch should have been preceded by the VIN driver patch and
>> bindings description, don't you think?

>>> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts
>>> b/arch/arm/boot/dts/r8a7790-lager.dts
>>> index a087421..7528cfc 100644
>>> --- a/arch/arm/boot/dts/r8a7790-lager.dts
>>> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
>> [...]
>>> @@ -239,8 +244,41 @@
>>>       status = "ok";
>>>       pinctrl-0 = <&i2c2_pins>;
>>>       pinctrl-names = "default";
>>> +
>>> +    adv7180: adv7180@0x20 {

>>     ePAPR standard [1] tells us that:

>> "The name of a node should be somewhat generic, reflecting the function
>> of the device and not its precise programming model."

>>     So, I would suggest something like "video-decoder" instead. And
>> remove "0x" from the address part of the node name please.

> Personally I'm not fussed about names in the tree.

    I quoted the standard everybody should follow.

> I will look at changing it to something like vin1_decoder

    Please give it a generic name instead, without indexes and underscores.

WBR, Sergei

Re: [PATCH 5/5] rcar_vin: add devicetree support

[Guennadi Liakhovetski]

Hi Ben,

On Fri, 7 Mar 2014, Ben Dooks wrote:

> Add support for devicetree probe for the rcar-vin
> driver.

Thanks for the patch. I'm afraid I don't quite understand how it is 
supposed to work though. AFAICS, the rcar_vin driver currently doesn't 
support asynchronous probing, so, your code is relying on a specific 
probing order? Besides, as of now soc-camera doesn't support OF probing. 
You mentioned, that you have a patch for soc-camera to add OF support. I 
think that and async probing for rcar_vin should be committed before this 
patch becomes meaningful.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 2/5] ARM: lager: add vin1 node

[Simon Horman]

On Fri, Mar 07, 2014 at 01:01:36PM +0000, Ben Dooks wrote:
> Add device-tree for vin1 (composite video in) on the
> lager board.

Please update the prefix of the subject of this patch to:
ARM: shmobile: lager: 

> 
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
>  arch/arm/boot/dts/r8a7790-lager.dts | 38 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index a087421..7528cfc 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -158,6 +158,11 @@
>  		renesas,groups = "i2c2";
>  		renesas,function = "i2c2";
>  	};
> +
> +	vin1_pins: vin {
> +		renesas,groups = "vin1_data8", "vin1_clk";
> +		renesas,function = "vin1";
> +	};
>  };
>  
>  &mmcif1 {
> @@ -239,8 +244,41 @@
>  	status = "ok";
>  	pinctrl-0 = <&i2c2_pins>;
>  	pinctrl-names = "default";
> +
> +	adv7180: adv7180@0x20 {
> +		compatible = "adi,adv7180";
> +		reg = <0x20>;
> +		remote = <&vin1>;
> +
> +		port {
> +			adv7180_1: endpoint {
> +				bus-width = <8>;
> +				remote-endpoint = <&vin1ep0>;
> +			};
> +		};
> +	};
> +
>  };
>  
>  &i2c3	{
>  	status = "ok";
>  };
> +
> +/* composite video input */
> +&vin1 {
> +	pinctrl-0 = <&vin1_pins>;
> +	pinctrl-names = "default";
> +
> +	status = "ok";
> +
> +	port {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		vin1ep0: endpoint {
> +			remote-endpoint = <&adv7180_1>;
> +			bus-width = <8>;
> +		};
> +	};
> +};
> +
> -- 
> 1.9.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/5] r8a7790.dtsi: add vin[0-3] nodes

[Simon Horman]

On Fri, Mar 07, 2014 at 01:01:35PM +0000, Ben Dooks wrote:
> Add nodes for the four video input channels on the R8A7790.

Please update the prefix of this subject of this patch to:
ARM: shmobile: r8a7790: 

> 
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
>  arch/arm/boot/dts/r8a7790.dtsi | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index a1e7c39..4c3eafb 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -395,6 +395,38 @@
>  		status = "disabled";
>  	};
>  
> +	vin0: vin@0xe6ef0000 {
> +		compatible = "renesas,vin-r8a7790";
> +		clocks = <&mstp8_clks R8A7790_CLK_VIN0>;
> +		reg = <0 0xe6ef0000 0 0x1000>;
> +		interrupts = <0 188 IRQ_TYPE_LEVEL_HIGH>;
> +		status = "disabled";
> +	};
> +
> +	vin1: vin@0xe6ef1000 {
> +		compatible = "renesas,vin-r8a7790";
> +		clocks = <&mstp8_clks R8A7790_CLK_VIN1>;
> +		reg = <0 0xe6ef1000 0 0x1000>;
> +		interrupts = <0 189 IRQ_TYPE_LEVEL_HIGH>;
> +		status = "disabled";
> +	};
> +
> +	vin2: vin@0xe6ef2000 {
> +		compatible = "renesas,vin-r8a7790";
> +		clocks = <&mstp8_clks R8A7790_CLK_VIN2>;
> +		reg = <0 0xe6ef2000 0 0x1000>;
> +		interrupts = <0 190 IRQ_TYPE_LEVEL_HIGH>;
> +		status = "disabled";
> +	};
> +
> +	vin3: vin@0xe6ef3000 {
> +		compatible = "renesas,vin-r8a7790";
> +		clocks = <&mstp8_clks R8A7790_CLK_VIN3>;
> +		reg = <0 0xe6ef3000 0 0x1000>;
> +		interrupts = <0 191 IRQ_TYPE_LEVEL_HIGH>;
> +		status = "disabled";
> +	};
> +
>  	clocks {
>  		#address-cells = <2>;
>  		#size-cells = <2>;
> -- 
> 1.9.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 5/5] rcar_vin: add devicetree support

[Guennadi Liakhovetski]

Hi Ben,

Since I never received a reply to this my query, I consider this your 
patch series suspended.

Thanks
Guennadi

On Sat, 8 Mar 2014, Guennadi Liakhovetski wrote:

> Hi Ben,
> 
> On Fri, 7 Mar 2014, Ben Dooks wrote:
> 
> > Add support for devicetree probe for the rcar-vin
> > driver.
> 
> Thanks for the patch. I'm afraid I don't quite understand how it is 
> supposed to work though. AFAICS, the rcar_vin driver currently doesn't 
> support asynchronous probing, so, your code is relying on a specific 
> probing order? Besides, as of now soc-camera doesn't support OF probing. 
> You mentioned, that you have a patch for soc-camera to add OF support. I 
> think that and async probing for rcar_vin should be committed before this 
> patch becomes meaningful.

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 5/5] rcar_vin: add devicetree support

[Ben Dooks]

On 30/03/14 22:04, Guennadi Liakhovetski wrote:
> Hi Ben,
>
> Since I never received a reply to this my query, I consider this your
> patch series suspended.
>
> Thanks
> Guennadi

I meant to send out a patch series for the of probe for soc_camera.
The actual rcar_vin does not need much to support async probe, it is
just the soc_camera that needs sorting.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH 5/5] rcar_vin: add devicetree support

[Guennadi Liakhovetski]

On Sun, 30 Mar 2014, Ben Dooks wrote:

> On 30/03/14 22:04, Guennadi Liakhovetski wrote:
> > Hi Ben,
> > 
> > Since I never received a reply to this my query, I consider this your
> > patch series suspended.
> > 
> > Thanks
> > Guennadi
> 
> I meant to send out a patch series for the of probe for soc_camera.
> The actual rcar_vin does not need much to support async probe, it is
> just the soc_camera that needs sorting.

But without that soc-camera DT support your patches are non-functional, 
right?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 5/5] rcar_vin: add devicetree support

[Ben Dooks]

On 30/03/14 22:17, Guennadi Liakhovetski wrote:
> On Sun, 30 Mar 2014, Ben Dooks wrote:
>
>> On 30/03/14 22:04, Guennadi Liakhovetski wrote:
>>> Hi Ben,
>>>
>>> Since I never received a reply to this my query, I consider this your
>>> patch series suspended.
>>>
>>> Thanks
>>> Guennadi
>>
>> I meant to send out a patch series for the of probe for soc_camera.
>> The actual rcar_vin does not need much to support async probe, it is
>> just the soc_camera that needs sorting.
>
> But without that soc-camera DT support your patches are non-functional,
> right?

They're not much use. I've sent the soc-camera patch out now for
review. I will sort out sending patches for the async device changes
for the device(s) on the lager board as soon as I can get some time
to re-test.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius