All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers
@ 2012-12-31 16:02 Sylwester Nawrocki
  2012-12-31 16:02 ` [PATCH RFC v2 01/15] [media] Add common video interfaces OF bindings documentation Sylwester Nawrocki
                   ` (14 more replies)
  0 siblings, 15 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:02 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

This series includes the updated DT bindings documentation and the V4L2 OF
parser. Changes since v1:

 - renamed 'link' nodes to 'endpoint', remote phandle to 'remote-endpoint'
   in the common bindings documentation file,
 - removed references to V4L2 in the documentation,
 - file v4l2.txt renamed to video-interfaces.txt,
 - dropped patches adding empty function definitions for when CONFIG_OF
   is disabled,
 - v4l2_of_parse_link() function renamed to v4l2_of_parse_endpoint,
 - created separate helpers to parse parallel and MIPI-CSI2 bus properties
   independently.

The bindings documentation patch is almost unchanged since version posted
on 12 Dec [1].

This series also includes patches adding device tree support for Exynos4
SoC camera subsystem drivers. Changes in that part include:

 - code refactoring to not depend on dummy function definitions,
 - updated to use new helpers from drivers/media/v4l2-core/v4l2-of.c,
 - added parsing of nodes corresponding to parallel video bus sensors.

My next steps include testing this on top of the common clock framework
patches for Exynos SoCs, posted recently by Thomas Abraham, and further
adding support for v4l2 asynchronous sub-device registration. However
I'd like to possibly have these patches in mainline first, only except
the "ARM: EXYNOS4: Add OF_DEV_AUXDATA for FIMC, FIMC-LITE and CSIS" one,
which won't be needed when there is common clock support available.

I appreciate any feedback, especially on the documentation and the OF
helpers.

Full tree containing all patches can be browsed at [2].

[1] http://patchwork.linuxtv.org/patch/15911/
[2] http://git.infradead.org/users/kmpark/linux-samsung/shortlog/refs/heads/v3.7-pq-camera-dt


Guennadi Liakhovetski (2):
  [media] Add common video interfaces OF bindings documentation
  [media] Add a V4L2 OF parser

Sylwester Nawrocki (13):
  s5p-csis: Add device tree support
  s5p-fimc: Support for FIMC devices instantiated from the device tree
  s5p-fimc: Support for FIMC-LITE devices instantiated from the device
    tree
  s5p-fimc: Change platform subdevs registration method
  s5p-fimc: Support camera media device initialization on DT systems
  s5p-fimc: Add device tree based sensors registration
  s5p-fimc: Use pinctrl API for camera ports configuration
  ARM: EXYNOS4: Add OF_DEV_AUXDATA for FIMC, FIMC-LITE and CSIS
  ARM: dts: Add camera node exynos4.dtsi
  ARM: dts: Add ISP power domain node for Exynos4x12
  ARM: dts: Add FIMC and MIPI CSIS device nodes for Exynos4x12
  ARM: dts: Add camera pinctrl nodes for Exynos4x12 SoCs
  ARM: dts: Add camera device nodes nodes for PQ board

 .../devicetree/bindings/media/soc/samsung-fimc.txt |  183 +++++++
 .../bindings/media/soc/samsung-mipi-csis.txt       |   84 ++++
 .../devicetree/bindings/media/video-interfaces.txt |  198 ++++++++
 arch/arm/boot/dts/exynos4.dtsi                     |   64 +++
 arch/arm/boot/dts/exynos4412-slp_pq.dts            |  133 +++++
 arch/arm/boot/dts/exynos4x12-pinctrl.dtsi          |   33 +-
 arch/arm/boot/dts/exynos4x12.dtsi                  |   52 ++
 arch/arm/mach-exynos/mach-exynos4-dt.c             |   16 +
 drivers/media/platform/s5p-fimc/fimc-capture.c     |    2 +-
 drivers/media/platform/s5p-fimc/fimc-core.c        |   85 ++--
 drivers/media/platform/s5p-fimc/fimc-lite.c        |   65 ++-
 drivers/media/platform/s5p-fimc/fimc-mdevice.c     |  529 +++++++++++++++-----
 drivers/media/platform/s5p-fimc/fimc-mdevice.h     |   10 +
 drivers/media/platform/s5p-fimc/mipi-csis.c        |  159 ++++--
 drivers/media/platform/s5p-fimc/mipi-csis.h        |    1 +
 drivers/media/v4l2-core/Makefile                   |    3 +
 drivers/media/v4l2-core/v4l2-of.c                  |  249 +++++++++
 include/media/s5p_fimc.h                           |   16 +
 include/media/v4l2-of.h                            |   79 +++
 19 files changed, 1739 insertions(+), 222 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/soc/samsung-fimc.txt
 create mode 100644 Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt
 create mode 100644 Documentation/devicetree/bindings/media/video-interfaces.txt
 create mode 100644 drivers/media/v4l2-core/v4l2-of.c
 create mode 100644 include/media/v4l2-of.h

--
1.7.9.5

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

* [PATCH RFC v2 01/15] [media] Add common video interfaces OF bindings documentation
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
@ 2012-12-31 16:02 ` Sylwester Nawrocki
  2013-01-02 11:31   ` Guennadi Liakhovetski
  2013-01-03 17:03   ` [PATCH RFC v3 " Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 02/15] [media] Add a V4L2 OF parser Sylwester Nawrocki
                   ` (13 subsequent siblings)
  14 siblings, 2 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:02 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

This patch adds a document describing common OF bindings for video
capture, output and video processing devices. It is curently mainly
focused on video capture devices, with data busses defined by
standards like ITU-R BT.656 or MIPI-CSI2.
It also documents a method of describing data links between devices.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>

---

This is basically a resend of my previous version of this patch [1],
with just a few typo/grammar issue corrections.

[1] http://patchwork.linuxtv.org/patch/15911/
---
 .../devicetree/bindings/media/video-interfaces.txt |  198 ++++++++++++++++++++
 1 file changed, 198 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/video-interfaces.txt

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
new file mode 100644
index 0000000..d1eea35
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -0,0 +1,198 @@
+Common bindings for video data receiver and transmitter interfaces
+
+General concept
+---------------
+
+Video data pipelines usually consist of external devices, e.g. camera sensors,
+controlled over an I2C, SPI or UART bus, and SoC internal IP blocks, including
+video DMA engines and video data processors.
+
+SoC internal blocks are described by DT nodes, placed similarly to other SoC
+blocks.  External devices are represented as child nodes of their respective
+bus controller nodes, e.g. I2C.
+
+Data interfaces on all video devices are described by their child 'port' nodes.
+Configuration of a port depends on other devices participating in the data
+transfer and is described by 'endpoint' subnodes.
+
+dev {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	port@0 {
+		endpoint@0 { ... };
+		endpoint@1 { ... };
+	};
+	port@1 { ... };
+};
+
+If a port can be configured to work with more than one other device on the same
+bus, an 'endpoint' child node must be provided for each of them.  If more than
+one port is present in a device node or there is more than one endpoint at a
+port, a common scheme, using '#address-cells', '#size-cells' and 'reg' properties
+is used.
+
+Two 'endpoint' nodes are linked with each other through their 'remote-endpoint'
+phandles.  An endpoint subnode of a device contains all properties needed for
+configuration of this device for data exchange with the other device.  In most
+cases properties at the peer 'endpoint' nodes will be identical, however
+they might need to be different when there is any signal modifications on the
+bus between two devices, e.g. there are logic signal inverters on the lines.
+
+Required properties
+-------------------
+
+If there is more than one 'port' or more than one 'endpoint' node following
+properties are required in relevant parent node:
+
+- #address-cells : number of cells required to define port number, should be 1.
+- #size-cells    : should be zero.
+
+Optional endpoint properties
+----------------------------
+
+- remote-endpoint : phandle to an 'endpoint' subnode of the other device node.
+- slave-mode : a boolean property, run the link in slave mode. Default is master
+  mode.
+- bus-width : number of data lines, valid for parallel buses.
+- data-shift: on parallel data busses, if bus-width is used to specify the
+  number of data lines, data-shift can be used to specify which data lines are
+  used, e.g. "bus-width=<10>; data-shift=<2>;" means, that lines 9:2 are used.
+- hsync-active : active state of HSYNC signal, 0/1 for LOW/HIGH respectively.
+- vsync-active : active state of VSYNC signal, 0/1 for LOW/HIGH respectively.
+  Note, that if HSYNC and VSYNC polarities are not specified, embedded
+  synchronization may be required, where supported.
+- data-active : similar to HSYNC and VSYNC, specifies data line polarity.
+- field-even-active: field signal level during the even field data transmission.
+- pclk-sample : rising (1) or falling (0) edge to sample the pixel clock signal.
+- data-lanes : an array of physical data lane indexes. Position of an entry
+  determines logical lane number, while the value of an entry indicates physical
+  lane, e.g. for 2-lane MIPI CSI-2 bus we could have "data-lanes = <1>, <2>;",
+  assuming the clock lane is on hardware lane 0. This property is valid for
+  serial buses only (e.g. MIPI CSI-2).
+- clock-lanes : a number of physical lane used as a clock lane.
+- clock-noncontinuous : a boolean property to allow MIPI CSI-2 non-continuous
+  clock mode.
+
+Example
+-------
+
+The below example snippet describes two data pipelines.  ov772x and imx074 are
+camera sensors with parallel and serial (MIPI CSI-2) video bus respectively.
+Both sensors are on I2C control bus corresponding to i2c0 controller node.
+ov772x sensor is linked directly to the ceu0 video host interface.  imx074 is
+linked to ceu0 through MIPI CSI-2 receiver (csi2). ceu0 has a (single) DMA
+engine writing captured data to memory.  ceu0 node has single 'port' node which
+indicates at any time only one of following data pipeline can be active:
+ov772x -> ceu0 or imx074 -> csi2 -> ceu0.
+
+	ceu0: ceu@0xfe910000 {
+		compatible = "renesas,sh-mobile-ceu";
+		reg = <0xfe910000 0xa0>;
+		interrupts = <0x880>;
+
+		mclk: master_clock {
+			compatible = "renesas,ceu-clock";
+			#clock-cells = <1>;
+			clock-frequency = <50000000>;	/* Max clock frequency */
+			clock-output-names = "mclk";
+		};
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			ceu0_1: endpoint@1 {
+				reg = <1>;		/* Local endpoint # */
+				remote = <&ov772x_1_1>;	/* Remote phandle */
+				bus-width = <8>;	/* Used data lines */
+				data-shift = <0>;	/* Lines 7:0 are used */
+
+				/* If hsync-active/vsync-active are missing,
+				   embedded bt.605 sync is used */
+				hsync-active = <1>;	/* Active high */
+				vsync-active = <1>;	/* Active high */
+				data-active = <1>;	/* Active high */
+				pclk-sample = <1>;	/* Rising */
+			};
+
+			ceu0_0: endpoint@0 {
+				reg = <0>;
+				remote = <&csi2_2>;
+				immutable;
+			};
+		};
+	};
+
+	i2c0: i2c@0xfff20000 {
+		...
+		ov772x_1: camera@0x21 {
+			compatible = "omnivision,ov772x";
+			reg = <0x21>;
+			vddio-supply = <&regulator1>;
+			vddcore-supply = <&regulator2>;
+
+			clock-frequency = <20000000>;
+			clocks = <&mclk 0>;
+			clock-names = "xclk";
+
+			port {
+				/* With 1 endpoint per port no need in addresses. */
+				ov772x_1_1: endpoint {
+					bus-width = <8>;
+					remote-endpoint = <&ceu0_1>;
+					hsync-active = <1>;
+					vsync-active = <0>; /* Who came up with an
+							       inverter here ?... */
+					data-active = <1>;
+					pclk-sample = <1>;
+				};
+			};
+		};
+
+		imx074: camera@0x1a {
+			compatible = "sony,imx074";
+			reg = <0x1a>;
+			vddio-supply = <&regulator1>;
+			vddcore-supply = <&regulator2>;
+
+			clock-frequency = <30000000>;	/* Shared clock with ov772x_1 */
+			clocks = <&mclk 0>;
+			clock-names = "sysclk";		/* Assuming this is the
+							   name in the datasheet */
+			port {
+				imx074_1: endpoint {
+					clock-lanes = <0>;
+					data-lanes = <1>, <2>;
+					remote-endpoint = <&csi2_1>;
+				};
+			};
+		};
+	};
+
+	csi2: csi2@0xffc90000 {
+		compatible = "renesas,sh-mobile-csi2";
+		reg = <0xffc90000 0x1000>;
+		interrupts = <0x17a0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@1 {
+			compatible = "renesas,csi2c";	/* One of CSI2I and CSI2C. */
+			reg = <1>;			/* CSI-2 PHY #1 of 2: PHY_S,
+							   PHY_M has port address 0,
+							   is unused. */
+			csi2_1: endpoint {
+				clock-lanes = <0>;
+				data-lanes = <2>, <1>;
+				remote-endpoint = <&imx074_1>;
+			};
+		};
+		port@2 {
+			reg = <2>;			/* port 2: link to the CEU */
+
+			csi2_2: endpoint {
+				immutable;
+				remote-endpoint = <&ceu0_0>;
+			};
+		};
+	};
-- 
1.7.9.5

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

* [PATCH RFC v2 02/15] [media] Add a V4L2 OF parser
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
  2012-12-31 16:02 ` [PATCH RFC v2 01/15] [media] Add common video interfaces OF bindings documentation Sylwester Nawrocki
@ 2012-12-31 16:03 ` Sylwester Nawrocki
  2013-01-02 11:58   ` Guennadi Liakhovetski
       [not found]   ` <1356969793-27268-3-git-send-email-s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
  2012-12-31 16:03 ` [PATCH RFC v2 03/15] s5p-csis: Add device tree support Sylwester Nawrocki
                   ` (12 subsequent siblings)
  14 siblings, 2 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Add a V4L2 OF parser, implementing bindings, documented in
Documentation/devicetree/bindings/media/video-interfaces.txt.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---

Changes since previous version:
- merged into this one all my fixup patches from this series [1]:
  v4l2-of: Support variable length of data-lanes property
  v4l2-of: Add v4l2_of_parse_data_lanes() function
  v4l2-of: Corrected v4l2_of_parse_link() function declaration
  v4l2_of_parse_link() return value type is int, not void.
  v4l2-of: Replace "remote" property with "remote-endpoint"

[1] https://lkml.org/lkml/2012/12/10/464
---
 drivers/media/v4l2-core/Makefile  |    3 +
 drivers/media/v4l2-core/v4l2-of.c |  249 +++++++++++++++++++++++++++++++++++++
 include/media/v4l2-of.h           |   79 ++++++++++++
 3 files changed, 331 insertions(+)
 create mode 100644 drivers/media/v4l2-core/v4l2-of.c
 create mode 100644 include/media/v4l2-of.h

diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index c2d61d4..00f64d6 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -9,6 +9,9 @@ videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
 ifeq ($(CONFIG_COMPAT),y)
   videodev-objs += v4l2-compat-ioctl32.o
 endif
+ifeq ($(CONFIG_OF),y)
+  videodev-objs += v4l2-of.o
+endif
 
 obj-$(CONFIG_VIDEO_DEV) += videodev.o v4l2-int-device.o
 obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
new file mode 100644
index 0000000..cdac04b
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -0,0 +1,249 @@
+/*
+ * V4L2 OF binding parsing library
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include <media/v4l2-of.h>
+
+/**
+ * v4l2_of_parse_mipi_csi2() - parse MIPI CSI-2 bus properties
+ * @node: pointer to endpoint device_node
+ * @endpoint: pointer to v4l2_of_endpoint data structure
+ *
+ * Return: 0 on success or negative error value otherwise.
+ */
+int v4l2_of_parse_mipi_csi2(const struct device_node *node,
+			    struct v4l2_of_endpoint *endpoint)
+{
+	struct v4l2_of_mipi_csi2 *mipi_csi2 = &endpoint->mipi_csi_2;
+	u32 data_lanes[ARRAY_SIZE(mipi_csi2->data_lanes)];
+	struct property *prop;
+	const __be32 *lane = NULL;
+	u32 v;
+	int i = 0;
+
+	prop = of_find_property(node, "data-lanes", NULL);
+	if (!prop)
+		return -EINVAL;
+	do {
+		lane = of_prop_next_u32(prop, lane, &data_lanes[i]);
+	} while (lane && i++ < ARRAY_SIZE(data_lanes));
+
+	mipi_csi2->num_data_lanes = i;
+	while (i--)
+		mipi_csi2->data_lanes[i] = data_lanes[i];
+
+	if (!of_property_read_u32(node, "clock-lanes", &v))
+		mipi_csi2->clock_lane = v;
+
+	if (of_get_property(node, "clock-noncontinuous", &v))
+		endpoint->mbus_flags |= V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK;
+
+	return 0;
+}
+
+/**
+ * v4l2_of_parse_parallel_bus() - parse parallel bus properties
+ * @node: pointer to endpoint device_node
+ * @endpoint: pointer to v4l2_of_endpoint data structure
+ */
+void v4l2_of_parse_parallel_bus(const struct device_node *node,
+				struct v4l2_of_endpoint *endpoint)
+{
+	unsigned int flags = 0;
+	u32 v;
+
+	if (WARN_ON(!endpoint))
+		return;
+
+	if (!of_property_read_u32(node, "hsync-active", &v))
+		flags |= v ? V4L2_MBUS_HSYNC_ACTIVE_HIGH :
+			V4L2_MBUS_HSYNC_ACTIVE_LOW;
+
+	if (!of_property_read_u32(node, "vsync-active", &v))
+		flags |= v ? V4L2_MBUS_VSYNC_ACTIVE_HIGH :
+			V4L2_MBUS_VSYNC_ACTIVE_LOW;
+
+	if (!of_property_read_u32(node, "pclk-sample", &v))
+		flags |= v ? V4L2_MBUS_PCLK_SAMPLE_RISING :
+			V4L2_MBUS_PCLK_SAMPLE_FALLING;
+
+	if (!of_property_read_u32(node, "field-even-active", &v))
+		flags |= v ? V4L2_MBUS_FIELD_EVEN_HIGH :
+			V4L2_MBUS_FIELD_EVEN_LOW;
+	if (flags)
+		endpoint->mbus_type = V4L2_MBUS_PARALLEL;
+	else
+		endpoint->mbus_type = V4L2_MBUS_BT656;
+
+	if (!of_property_read_u32(node, "data-active", &v))
+		flags |= v ? V4L2_MBUS_DATA_ACTIVE_HIGH :
+			V4L2_MBUS_DATA_ACTIVE_LOW;
+
+	if (of_get_property(node, "slave-mode", &v))
+		flags |= V4L2_MBUS_SLAVE;
+
+	if (!of_property_read_u32(node, "bus-width", &v))
+		endpoint->parallel.bus_width = v;
+
+	if (!of_property_read_u32(node, "data-shift", &v))
+		endpoint->parallel.data_shift = v;
+
+	endpoint->mbus_flags = flags;
+}
+
+/**
+ * v4l2_of_parse_endpoint() - parse all endpoint node properties
+ * @node: pointer to endpoint device_node
+ * @endpoint: pointer to v4l2_of_endpoint data structure
+ *
+ * All properties are optional. If none are found, we don't set any flags.
+ * This means the port has a static configuration and no properties have
+ * to be specified explicitly.
+ * If any properties that identify the bus as parallel are found and
+ * slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if we recognise
+ * the bus as serial CSI-2 and clock-noncontinuous isn't set, we set the
+ * V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag.
+ * The caller should hold a reference to @node.
+ */
+void v4l2_of_parse_endpoint(const struct device_node *node,
+			    struct v4l2_of_endpoint *endpoint)
+{
+	const struct device_node *port_node = of_get_parent(node);
+	bool data_lanes_present = false;
+
+	memset(endpoint, 0, sizeof(*endpoint));
+
+	endpoint->local_node = node;
+
+	/* Doesn't matter, whether the below two calls succeed */
+	of_property_read_u32(port_node, "reg", &endpoint->port);
+	of_property_read_u32(node, "reg", &endpoint->addr);
+
+	v4l2_of_parse_parallel_bus(node, endpoint);
+
+	/* If any parallel bus properties have been found, skip serial ones */
+	if (endpoint->parallel.bus_width || endpoint->parallel.data_shift ||
+	    endpoint->mbus_flags) {
+		/* Default parallel bus-master */
+		if (!(endpoint->mbus_flags & V4L2_MBUS_SLAVE))
+			endpoint->mbus_flags |= V4L2_MBUS_MASTER;
+		return;
+	}
+
+	endpoint->mbus_type = V4L2_MBUS_CSI2;
+
+	if (!v4l2_of_parse_mipi_csi2(node, endpoint))
+		data_lanes_present = true;
+
+	if ((endpoint->mipi_csi_2.clock_lane || data_lanes_present) &&
+	    !(endpoint->mbus_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK)) {
+		/* Default CSI-2: continuous clock */
+		endpoint->mbus_flags |= V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+	}
+}
+EXPORT_SYMBOL(v4l2_of_parse_endpoint);
+
+/*
+ * Return a refcounted next "endpoint" DT node. Contrary to the common OF
+ * practice, we do not drop the reference to previous, users have to do it
+ * themselves, when they're done with the node.
+ */
+struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
+					struct device_node *previous)
+{
+	struct device_node *child, *port;
+
+	if (!parent)
+		return NULL;
+
+	if (!previous) {
+		/*
+		 * If this is the first call, we have to find a port within this
+		 * node
+		 */
+		for_each_child_of_node(parent, port) {
+			if (!of_node_cmp(port->name, "port"))
+				break;
+		}
+		if (port) {
+			/* Found a port, get a link */
+			child = of_get_next_child(port, NULL);
+			of_node_put(port);
+		} else {
+			child = NULL;
+		}
+		if (!child)
+			pr_err("%s(): Invalid DT: %s has no link children!\n",
+			       __func__, parent->name);
+	} else {
+		port = of_get_parent(previous);
+		if (!port)
+			/* Hm, has someone given us the root node?... */
+			return NULL;
+
+		/* Avoid dropping previous refcount to 0 */
+		of_node_get(previous);
+		child = of_get_next_child(port, previous);
+		if (child) {
+			of_node_put(port);
+			return child;
+		}
+
+		/* No more links under this port, try the next one */
+		do {
+			port = of_get_next_child(parent, port);
+			if (!port)
+				return NULL;
+		} while (of_node_cmp(port->name, "port"));
+
+		/* Pick up the first link on this port */
+		child = of_get_next_child(port, NULL);
+		of_node_put(port);
+	}
+
+	return child;
+}
+EXPORT_SYMBOL(v4l2_of_get_next_endpoint);
+
+/**
+ * v4l2_of_get_remote_port_parent() - get remote port's parent node
+ * @node: pointer to local endpoint device_node
+ *
+ * Return: Remote device node associated with remote endpoint node linked
+ *	   to @node. Use of_node_put() on it when done.
+ */
+struct device_node *v4l2_of_get_remote_port_parent(
+			       const struct device_node *node)
+{
+	struct device_node *re, *tmp;
+
+	/* Get remote endpoint DT node. */
+	re = of_parse_phandle(node, "remote-endpoint", 0);
+	if (!re)
+		return NULL;
+
+	/* Remote port. */
+	tmp = of_get_parent(re);
+	of_node_put(re);
+	if (!tmp)
+		return NULL;
+
+	/* Remote device node. */
+	re = of_get_parent(tmp);
+	of_node_put(tmp);
+
+	return re;
+}
+EXPORT_SYMBOL(v4l2_of_get_remote_port_parent);
diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
new file mode 100644
index 0000000..1aba3b3
--- /dev/null
+++ b/include/media/v4l2-of.h
@@ -0,0 +1,79 @@
+/*
+ * V4L2 OF binding parsing library
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#ifndef _V4L2_OF_H
+#define _V4L2_OF_H
+
+#include <linux/list.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+
+#include <media/v4l2-mediabus.h>
+
+struct device_node;
+
+struct v4l2_of_mipi_csi2 {
+	unsigned char data_lanes[4];
+	unsigned char clock_lane;
+	unsigned short num_data_lanes;
+};
+
+struct v4l2_of_endpoint {
+	unsigned int port;
+	unsigned int addr;
+	struct list_head head;
+	const struct device_node *local_node;
+	const __be32 *remote;
+	enum v4l2_mbus_type mbus_type;
+	unsigned int mbus_flags;
+	union {
+		struct {
+			unsigned char bus_width;
+			unsigned char data_shift;
+		} parallel;
+		struct v4l2_of_mipi_csi2 mipi_csi_2;
+	};
+};
+
+#ifdef CONFIG_OF
+int v4l2_of_parse_mipi_csi2(const struct device_node *node,
+			    struct v4l2_of_endpoint *endpoint);
+void v4l2_of_parse_parallel_bus(const struct device_node *node,
+				struct v4l2_of_endpoint *endpoint);
+void v4l2_of_parse_endpoint(const struct device_node *node,
+			    struct v4l2_of_endpoint *link);
+struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
+					struct device_node *previous);
+struct device_node *v4l2_of_get_remote_port_parent(
+					const struct device_node *node);
+#else /* CONFIG_OF */
+
+static inline int v4l2_of_parse_endpoint(const struct device_node *node,
+					struct v4l2_of_endpoint *link)
+{
+	return -ENOSYS;
+}
+
+static inline struct device_node *v4l2_of_get_next_endpoint(
+					const struct device_node *parent,
+					struct device_node *previous)
+{
+	return NULL;
+}
+
+static inline struct device_node *v4l2_of_get_remote_endpoint(
+					const struct device_node *node)
+{
+	return NULL;
+}
+
+#endif /* CONFIG_OF */
+
+#endif /* _V4L2_OF_H */
-- 
1.7.9.5

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

* [PATCH RFC v2 03/15] s5p-csis: Add device tree support
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
  2012-12-31 16:02 ` [PATCH RFC v2 01/15] [media] Add common video interfaces OF bindings documentation Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 02/15] [media] Add a V4L2 OF parser Sylwester Nawrocki
@ 2012-12-31 16:03 ` Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 04/15] s5p-fimc: Support for FIMC devices instantiated from the device tree Sylwester Nawrocki
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC
device. This patch support for binding the driver to the MIPI-CSIS
devices instantiated from device tree and for parsing all SoC and
board specific properties.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 .../bindings/media/soc/samsung-mipi-csis.txt       |   84 +++++++++++
 drivers/media/platform/s5p-fimc/mipi-csis.c        |  159 +++++++++++++++-----
 drivers/media/platform/s5p-fimc/mipi-csis.h        |    1 +
 3 files changed, 208 insertions(+), 36 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt

diff --git a/Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt b/Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt
new file mode 100644
index 0000000..c06ef5a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt
@@ -0,0 +1,84 @@
+Samsung S5P/EXYNOS SoC MIPI-CSI2 receiver (MIPI CSIS)
+-----------------------------------------------------
+
+Required properties:
+
+- compatible	  : "samsung,s5pv210-csis" for S5PV210 SoCs,
+		    "samsung,exynos4210-csis" for Exynos4210 and later SoCs;
+- reg		  : physical base address and size of the device memory mapped
+		    registers;
+- interrupts      : should contain MIPI CSIS interrupt; the format of the
+		    interrupt specifier depends on the interrupt controller;
+- max-data-lanes  : maximum number of data lanes supported (SoC specific);
+- vddio-supply    : MIPI CSIS I/O and PLL voltage supply (e.g. 1.8V);
+- vddcore-supply  : MIPI CSIS Core voltage supply (e.g. 1.1V).
+
+Optional properties:
+
+- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
+		    value when this property is not specified is 166 MHz;
+- samsung,csis,wclk : CSI-2 wrapper clock selection. If this property is present
+		    external clock from CMU will be used, if not bus clock will
+		    be selected.
+
+The device node should contain one 'port' child node with one child 'endpoint'
+node, as outlined in the common media bindings specification. See
+Documentation/devicetree/bindings/media/video-interfaces.txt for details.
+The following are properties specific to these nodes.
+
+port node
+---------
+
+- reg		  : (required) must be 3 for camera C input (CSIS0) or 4 for
+		    camera D input (CSIS1);
+
+endpoint node
+-------------
+
+- data-lanes	  : (required) an array specifying active physical MIPI-CSI2
+		    data input lanes and their mapping to logical lanes; the
+		    array's content is unused, only its length is meaningful;
+
+- samsung,csis-hs-settle : (optional) differential receiver (HS-RX) settle time;
+
+- samsung,camclk-out	 : (optional) specifies clock output for remote sensor,
+			   0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT;
+
+Example:
+
+	reg0: regulator@0 {
+	};
+
+	reg1: regulator@1 {
+	};
+
+/* SoC properties */
+
+	aliases {
+		csis0 = &csis_0;
+	};
+
+	csis_0: csis@11880000 {
+		compatible = "samsung,exynos4210-csis";
+		reg = <0x11880000 0x1000>;
+		interrupts = <0 78 0>;
+		max-data-lanes = <4>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+
+/* Board properties */
+
+	csis_0: csis@11880000 {
+		clock-frequency = <166000000>;
+		vddio-supply = <&reg0>;
+		vddcore-supply = <&reg1>;
+		port {
+			reg = <3>; /* 3 - CSIS0, 4 - CSIS1 */
+			csis0_ep: endpoint {
+				remote-endpoint = <...>;
+				data-lanes = <1>, <2>;
+				samsung,csis-hs-settle = <12>;
+			};
+		};
+	};
diff --git a/drivers/media/platform/s5p-fimc/mipi-csis.c b/drivers/media/platform/s5p-fimc/mipi-csis.c
index 8a06f14..31c25a6 100644
--- a/drivers/media/platform/s5p-fimc/mipi-csis.c
+++ b/drivers/media/platform/s5p-fimc/mipi-csis.c
@@ -19,12 +19,14 @@
 #include <linux/kernel.h>
 #include <linux/memory.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/videodev2.h>
+#include <media/v4l2-of.h>
 #include <media/v4l2-subdev.h>
 #include <linux/platform_data/mipi-csis.h>
 #include "mipi-csis.h"
@@ -113,6 +115,7 @@ static char *csi_clock_name[] = {
 	[CSIS_CLK_GATE] = "csis",
 };
 #define NUM_CSIS_CLOCKS	ARRAY_SIZE(csi_clock_name)
+#define DEFAULT_SCLK_CSIS_FREQ	166000000UL
 
 static const char * const csis_supply_name[] = {
 	"vddcore",  /* CSIS Core (1.0V, 1.1V or 1.2V) suppply */
@@ -167,6 +170,11 @@ struct csis_pktbuf {
  * @clock: CSIS clocks
  * @irq: requested s5p-mipi-csis irq number
  * @flags: the state variable for power and streaming control
+ * @clock_frequency: device bus clock frequency
+ * @hs_settle: HS-RX settle time
+ * @num_lanes: number of MIPI-CSI data lanes used
+ * @max_num_lanes: maximum number of MIPI-CSI data lanes supported
+ * @wclk_ext: CSI wrapper clock: 0 - bus clock, 1 - external SCLK_CAM
  * @csis_fmt: current CSIS pixel format
  * @format: common media bus format for the source and sink pad
  * @slock: spinlock protecting structure members below
@@ -184,6 +192,13 @@ struct csis_state {
 	struct clk *clock[NUM_CSIS_CLOCKS];
 	int irq;
 	u32 flags;
+
+	u32 clk_frequency;
+	u32 hs_settle;
+	u32 num_lanes;
+	u32 max_num_lanes;
+	u8 wclk_ext;
+
 	const struct csis_pix_format *csis_fmt;
 	struct v4l2_mbus_framefmt format;
 
@@ -273,7 +288,6 @@ static void s5pcsis_reset(struct csis_state *state)
 
 static void s5pcsis_system_enable(struct csis_state *state, int on)
 {
-	struct s5p_platform_mipi_csis *pdata = state->pdev->dev.platform_data;
 	u32 val, mask;
 
 	val = s5pcsis_read(state, S5PCSIS_CTRL);
@@ -286,7 +300,7 @@ static void s5pcsis_system_enable(struct csis_state *state, int on)
 	val = s5pcsis_read(state, S5PCSIS_DPHYCTRL);
 	val &= ~S5PCSIS_DPHYCTRL_ENABLE;
 	if (on) {
-		mask = (1 << (pdata->lanes + 1)) - 1;
+		mask = (1 << (state->num_lanes + 1)) - 1;
 		val |= (mask & S5PCSIS_DPHYCTRL_ENABLE);
 	}
 	s5pcsis_write(state, S5PCSIS_DPHYCTRL, val);
@@ -321,15 +335,14 @@ static void s5pcsis_set_hsync_settle(struct csis_state *state, int settle)
 
 static void s5pcsis_set_params(struct csis_state *state)
 {
-	struct s5p_platform_mipi_csis *pdata = state->pdev->dev.platform_data;
 	u32 val;
 
 	val = s5pcsis_read(state, S5PCSIS_CONFIG);
-	val = (val & ~S5PCSIS_CFG_NR_LANE_MASK) | (pdata->lanes - 1);
+	val = (val & ~S5PCSIS_CFG_NR_LANE_MASK) | (state->num_lanes - 1);
 	s5pcsis_write(state, S5PCSIS_CONFIG, val);
 
 	__s5pcsis_set_format(state);
-	s5pcsis_set_hsync_settle(state, pdata->hs_settle);
+	s5pcsis_set_hsync_settle(state, state->hs_settle);
 
 	val = s5pcsis_read(state, S5PCSIS_CTRL);
 	if (state->csis_fmt->data_alignment == 32)
@@ -338,7 +351,7 @@ static void s5pcsis_set_params(struct csis_state *state)
 		val &= ~S5PCSIS_CTRL_ALIGN_32BIT;
 
 	val &= ~S5PCSIS_CTRL_WCLK_EXTCLK;
-	if (pdata->wclk_source)
+	if (state->wclk_ext)
 		val |= S5PCSIS_CTRL_WCLK_EXTCLK;
 	s5pcsis_write(state, S5PCSIS_CTRL, val);
 
@@ -696,55 +709,118 @@ static irqreturn_t s5pcsis_irq_handler(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static int s5pcsis_get_platform_data(struct platform_device *pdev,
+				     struct csis_state *state)
+{
+	struct s5p_platform_mipi_csis *pdata = pdev->dev.platform_data;
+
+	if (pdata == NULL) {
+		dev_err(&pdev->dev, "Platform data not specified\n");
+		return -EINVAL;
+	}
+	state->clk_frequency = pdata->clk_rate;
+	state->num_lanes = pdata->lanes;
+	state->hs_settle = pdata->hs_settle;
+	state->index = max(0, pdev->id);
+	if (state->index == 1)
+		state->max_num_lanes = CSIS1_MAX_LANES;
+	else
+		state->max_num_lanes = CSIS0_MAX_LANES;
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static int s5pcsis_parse_dt(struct platform_device *pdev,
+			    struct csis_state *state)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct v4l2_of_endpoint endpoint;
+	int ret;
+
+	state->index = of_alias_get_id(node, "csis");
+	if (state->index < 0 || state->index >= CSIS_MAX_ENTITIES)
+		return -EINVAL;
+
+	ret = of_property_read_u32(node, "max-data-lanes",
+				   &state->max_num_lanes);
+	if (ret < 0)
+		return ret;
+
+	if (of_property_read_u32(node, "clock-frequency",
+				 &state->clk_frequency))
+		state->clk_frequency = DEFAULT_SCLK_CSIS_FREQ;
+
+	state->wclk_ext = of_property_read_bool(node,
+						"samsung,csis-wclk");
+	/*
+	 * Get endpoint node. We care only about first child
+	 * nodes since these are the only ones available.
+	 */
+	while ((node = of_get_next_child(node, NULL))) {
+		if (!of_node_cmp(node->name, "endpoint"))
+			break;
+		of_node_put(node);
+	};
+	if (!node)
+		return -EINVAL;
+	of_property_read_u32(node, "samsung,csis-hs-settle",
+			     &state->hs_settle);
+
+	ret = v4l2_of_parse_mipi_csi2(node, &endpoint);
+	state->num_lanes = endpoint.mipi_csi_2.num_data_lanes;
+	of_node_put(node);
+	return 0;
+}
+#else
+#define s5pcsis_parse_dt(pdev, state) (-ENOSYS)
+#endif
+
 static int __devinit s5pcsis_probe(struct platform_device *pdev)
 {
-	struct s5p_platform_mipi_csis *pdata;
+	struct device *dev = &pdev->dev;
 	struct resource *mem_res;
 	struct csis_state *state;
 	int ret = -ENOMEM;
 	int i;
 
-	state = devm_kzalloc(&pdev->dev, sizeof(*state), GFP_KERNEL);
+	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
 	if (!state)
 		return -ENOMEM;
 
 	mutex_init(&state->lock);
 	spin_lock_init(&state->slock);
-
 	state->pdev = pdev;
-	state->index = max(0, pdev->id);
 
-	pdata = pdev->dev.platform_data;
-	if (pdata == NULL) {
-		dev_err(&pdev->dev, "Platform data not fully specified\n");
-		return -EINVAL;
-	}
+	if (dev->of_node)
+		ret = s5pcsis_parse_dt(pdev, state);
+	else
+		ret = s5pcsis_get_platform_data(pdev, state);
+	if (ret < 0)
+		return ret;
 
-	if ((state->index == 1 && pdata->lanes > CSIS1_MAX_LANES) ||
-	    pdata->lanes > CSIS0_MAX_LANES) {
-		dev_err(&pdev->dev, "Unsupported number of data lanes: %d\n",
-			pdata->lanes);
+	if (state->num_lanes == 0 || state->num_lanes > state->max_num_lanes) {
+		dev_err(dev, "Unsupported number of data lanes: %d (max. %d)\n",
+			state->num_lanes, state->max_num_lanes);
 		return -EINVAL;
 	}
 
 	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	state->regs = devm_request_and_ioremap(&pdev->dev, mem_res);
+	state->regs = devm_request_and_ioremap(dev, mem_res);
 	if (state->regs == NULL) {
-		dev_err(&pdev->dev, "Failed to request and remap io memory\n");
+		dev_err(dev, "Failed to request and remap io memory\n");
 		return -ENXIO;
 	}
 
 	state->irq = platform_get_irq(pdev, 0);
 	if (state->irq < 0) {
-		dev_err(&pdev->dev, "Failed to get irq\n");
+		dev_err(dev, "Failed to get irq\n");
 		return state->irq;
 	}
 
 	for (i = 0; i < CSIS_NUM_SUPPLIES; i++)
 		state->supplies[i].supply = csis_supply_name[i];
 
-	ret = regulator_bulk_get(&pdev->dev, CSIS_NUM_SUPPLIES,
-				 state->supplies);
+	ret = regulator_bulk_get(dev, CSIS_NUM_SUPPLIES, state->supplies);
 	if (ret)
 		return ret;
 
@@ -753,21 +829,22 @@ static int __devinit s5pcsis_probe(struct platform_device *pdev)
 		goto e_clkput;
 
 	clk_enable(state->clock[CSIS_CLK_MUX]);
-	if (pdata->clk_rate)
-		clk_set_rate(state->clock[CSIS_CLK_MUX], pdata->clk_rate);
+	if (state->clk_frequency)
+		clk_set_rate(state->clock[CSIS_CLK_MUX], state->clk_frequency);
 	else
-		dev_WARN(&pdev->dev, "No clock frequency specified!\n");
+		dev_WARN(dev, "No clock frequency specified!\n");
 
-	ret = devm_request_irq(&pdev->dev, state->irq, s5pcsis_irq_handler,
-			       0, dev_name(&pdev->dev), state);
+	ret = devm_request_irq(dev, state->irq, s5pcsis_irq_handler,
+			       0, dev_name(dev), state);
 	if (ret) {
-		dev_err(&pdev->dev, "Interrupt request failed\n");
+		dev_err(dev, "Interrupt request failed\n");
 		goto e_regput;
 	}
 
 	v4l2_subdev_init(&state->sd, &s5pcsis_subdev_ops);
 	state->sd.owner = THIS_MODULE;
-	strlcpy(state->sd.name, dev_name(&pdev->dev), sizeof(state->sd.name));
+	snprintf(state->sd.name, sizeof(state->sd.name), "%s.%d",
+		 CSIS_SUBDEV_NAME, state->index);
 	state->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	state->csis_fmt = &s5pcsis_formats[0];
 
@@ -787,10 +864,12 @@ static int __devinit s5pcsis_probe(struct platform_device *pdev)
 
 	/* .. and a pointer to the subdev. */
 	platform_set_drvdata(pdev, &state->sd);
-
 	memcpy(state->events, s5pcsis_events, sizeof(state->events));
+	pm_runtime_enable(dev);
 
-	pm_runtime_enable(&pdev->dev);
+	dev_info(&pdev->dev, "lanes: %d, hs_settle: %d, wclk: %d, freq: %u\n",
+		 state->num_lanes, state->hs_settle, state->wclk_ext,
+		 state->clk_frequency);
 	return 0;
 
 e_regput:
@@ -916,13 +995,21 @@ static const struct dev_pm_ops s5pcsis_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(s5pcsis_suspend, s5pcsis_resume)
 };
 
+static const struct of_device_id s5pcsis_of_match[] __devinitconst = {
+	{ .compatible = "samsung,s5pv210-csis" },
+	{ .compatible = "samsung,exynos4210-csis" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, s5pcsis_of_match);
+
 static struct platform_driver s5pcsis_driver = {
 	.probe		= s5pcsis_probe,
 	.remove		= __devexit_p(s5pcsis_remove),
 	.driver		= {
-		.name	= CSIS_DRIVER_NAME,
-		.owner	= THIS_MODULE,
-		.pm	= &s5pcsis_pm_ops,
+		.of_match_table = of_match_ptr(s5pcsis_of_match),
+		.name		= CSIS_DRIVER_NAME,
+		.owner		= THIS_MODULE,
+		.pm		= &s5pcsis_pm_ops,
 	},
 };
 
diff --git a/drivers/media/platform/s5p-fimc/mipi-csis.h b/drivers/media/platform/s5p-fimc/mipi-csis.h
index 2709286..28c11c4 100644
--- a/drivers/media/platform/s5p-fimc/mipi-csis.h
+++ b/drivers/media/platform/s5p-fimc/mipi-csis.h
@@ -11,6 +11,7 @@
 #define S5P_MIPI_CSIS_H_
 
 #define CSIS_DRIVER_NAME	"s5p-mipi-csis"
+#define CSIS_SUBDEV_NAME	CSIS_DRIVER_NAME
 #define CSIS_MAX_ENTITIES	2
 #define CSIS0_MAX_LANES		4
 #define CSIS1_MAX_LANES		2
-- 
1.7.9.5

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

* [PATCH RFC v2 04/15] s5p-fimc: Support for FIMC devices instantiated from the device tree
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
                   ` (2 preceding siblings ...)
  2012-12-31 16:03 ` [PATCH RFC v2 03/15] s5p-csis: Add device tree support Sylwester Nawrocki
@ 2012-12-31 16:03 ` Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 05/15] s5p-fimc: Support for FIMC-LITE " Sylwester Nawrocki
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

This patch adds support for FIMC devices instantiated from devicetree
for S5PV210 and Exynos4 SoCs. The FIMC IP features include colorspace
conversion and scaling (mem-to-mem) and parallel/MIPI CSI2 bus video
capture interface.

Multiple SoC revision specific parameters are defined statically
in the driver and are used for both dt and non-dt. Specific driver
static data is selected based on the compatible property, and
previously platform device name was used to match driver data with
a specific SoC/IP version.

Aliases are used to determine an index of the IP which is essential
for linking FIMC IP with other ones, like MIPI-CSIS (MIPI CSI2 bus
frontend) or FIMC-LITE and FIMC-IS ISP.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 .../devicetree/bindings/media/soc/samsung-fimc.txt |   92 ++++++++++++++++++++
 drivers/media/platform/s5p-fimc/fimc-capture.c     |    2 +-
 drivers/media/platform/s5p-fimc/fimc-core.c        |   84 ++++++++++++------
 3 files changed, 148 insertions(+), 30 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/soc/samsung-fimc.txt

diff --git a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt
new file mode 100644
index 0000000..fab7e61
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt
@@ -0,0 +1,92 @@
+Samsung S5P/EXYNOS SoC Camera Subsystem (FIMC)
+----------------------------------------------
+
+The Exynos Camera subsystem comprises of multiple sub-devices that are
+represented by separate platform devices. Some of the IPs come in different
+variants accross the SoC revisions (FIMC) and some remain mostly unchanged
+(MIPI CSIS, FIMC-LITE).
+
+All those sub-subdevices are defined as parent nodes of the common device
+node, which also includes common properties of the whole subsystem not really
+specific to any single sub-device, like common camera port pins or external
+clocks for image sensors attached to the SoC.
+
+Common 'camera' node
+--------------------
+
+Required properties:
+
+- compatible	   : must be "samsung,fimc", "simple-bus"
+
+- pinctrl-names    : pinctrl names for camera port pinmux control, the values
+		     must be "default, "inactive".  "default" corresponds to
+		     pinmux configured for camera parallel bus; "inactive" is
+		     different from "default" only in that the CAMCLK pin is
+		     in high impedance state.
+- pinctrl-0..1	   : pinctrl properties corresponding to pinctrl-names
+
+The 'camera' node must include at least one 'fimc' child node.
+
+
+'fimc' device nodes
+-------------------
+
+Required properties:
+
+- compatible : "samsung,s5pv210-fimc" for S5PV210,
+	       "samsung,exynos4210-fimc" for Exynos4210,
+	       "samsung,exynos4212-fimc" for Exynos4212/4412 SoCs;
+- reg	     : physical base address and size of the device memory mapped
+	       registers;
+- interrupts : FIMC interrupt to the CPU should be described here;
+
+For every fimc node a numbered alias should be present in the aliases node.
+Aliases are of the form fimc<n>, where <n> is an integer (0...N) specifying
+the IP's instance index.
+
+'parallel-ports' node
+-----------------------
+
+This node should contain child 'port' nodes specifying active parallel video
+input ports. It includes camera A and camera B inputs. 'reg' property in the
+port nodes specifies the input - 0, 1 indicates input A, B respectively.
+
+Optional properties
+
+- samsung,camclk-out	 : specifies clock output for remote sensor,
+			   0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT;
+
+
+Example:
+
+	aliases {
+		csis0 = &csis_0;
+		fimc0 = &fimc_0;
+	};
+
+	camera {
+		compatible = "samsung,fimc", "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		status = "okay";
+
+		pinctrl-names = "default", "inactive";
+		pinctrl-0 = <&cam_port_a_clk_active>;
+		pinctrl-1 = <&cam_port_a_clk_idle>;
+
+		fimc_0: fimc@11800000 {
+			compatible = "samsung,exynos4210-fimc";
+			reg = <0x11800000 0x1000>;
+			interrupts = <0 85 0>;
+			status = "okay";
+		};
+
+		csis_0: csis@11880000 {
+			compatible = "samsung,exynos4210-csis";
+			reg = <0x11880000 0x1000>;
+			interrupts = <0 78 0>;
+			max-data-lanes = <4>;
+		};
+	};
+
+[1] Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt
diff --git a/drivers/media/platform/s5p-fimc/fimc-capture.c b/drivers/media/platform/s5p-fimc/fimc-capture.c
index 18a70e4..e716753 100644
--- a/drivers/media/platform/s5p-fimc/fimc-capture.c
+++ b/drivers/media/platform/s5p-fimc/fimc-capture.c
@@ -1888,7 +1888,7 @@ int fimc_initialize_capture_subdev(struct fimc_dev *fimc)
 
 	v4l2_subdev_init(sd, &fimc_subdev_ops);
 	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
-	snprintf(sd->name, sizeof(sd->name), "FIMC.%d", fimc->pdev->id);
+	snprintf(sd->name, sizeof(sd->name), "FIMC.%d", fimc->id);
 
 	fimc->vid_cap.sd_pads[FIMC_SD_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
 	fimc->vid_cap.sd_pads[FIMC_SD_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
diff --git a/drivers/media/platform/s5p-fimc/fimc-core.c b/drivers/media/platform/s5p-fimc/fimc-core.c
index 2a1558a..e7eabb7 100644
--- a/drivers/media/platform/s5p-fimc/fimc-core.c
+++ b/drivers/media/platform/s5p-fimc/fimc-core.c
@@ -21,6 +21,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/list.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/slab.h>
 #include <linux/clk.h>
 #include <media/v4l2-ioctl.h>
@@ -879,45 +881,54 @@ static int fimc_m2m_resume(struct fimc_dev *fimc)
 	return 0;
 }
 
+static const struct of_device_id fimc_of_match[];
+
 static int fimc_probe(struct platform_device *pdev)
 {
-	const struct fimc_drvdata *drv_data = fimc_get_drvdata(pdev);
-	struct s5p_platform_fimc *pdata;
+	struct fimc_drvdata *drv_data = NULL;
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *of_id;
 	struct fimc_dev *fimc;
 	struct resource *res;
 	int ret = 0;
 
-	if (pdev->id >= drv_data->num_entities) {
-		dev_err(&pdev->dev, "Invalid platform device id: %d\n",
-			pdev->id);
-		return -EINVAL;
-	}
-
-	fimc = devm_kzalloc(&pdev->dev, sizeof(*fimc), GFP_KERNEL);
+	fimc = devm_kzalloc(dev, sizeof(*fimc), GFP_KERNEL);
 	if (!fimc)
 		return -ENOMEM;
 
-	fimc->id = pdev->id;
+	if (dev->of_node) {
+		of_id = of_match_node(fimc_of_match, dev->of_node);
+		if (of_id)
+			drv_data = (struct fimc_drvdata *)of_id->data;
+
+		fimc->id = of_alias_get_id(dev->of_node, "fimc");
+	} else {
+		drv_data = fimc_get_drvdata(pdev);
+		fimc->id = pdev->id;
+	}
+
+	if (!drv_data || fimc->id < 0 || fimc->id >= drv_data->num_entities) {
+		dev_err(dev, "Invalid driver data or device index (%d)\n",
+			fimc->id);
+		return -EINVAL;
+	}
 
 	fimc->variant = drv_data->variant[fimc->id];
 	fimc->pdev = pdev;
-	pdata = pdev->dev.platform_data;
-	fimc->pdata = pdata;
-
 	init_waitqueue_head(&fimc->irq_queue);
 	spin_lock_init(&fimc->slock);
 	mutex_init(&fimc->lock);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	fimc->regs = devm_request_and_ioremap(&pdev->dev, res);
+	fimc->regs = devm_request_and_ioremap(dev, res);
 	if (fimc->regs == NULL) {
-		dev_err(&pdev->dev, "Failed to obtain io memory\n");
+		dev_err(dev, "Failed to obtain io memory\n");
 		return -ENOENT;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (res == NULL) {
-		dev_err(&pdev->dev, "Failed to get IRQ resource\n");
+		dev_err(dev, "Failed to get IRQ resource\n");
 		return -ENXIO;
 	}
 
@@ -927,10 +938,10 @@ static int fimc_probe(struct platform_device *pdev)
 	clk_set_rate(fimc->clock[CLK_BUS], drv_data->lclk_frequency);
 	clk_enable(fimc->clock[CLK_BUS]);
 
-	ret = devm_request_irq(&pdev->dev, res->start, fimc_irq_handler,
-			       0, dev_name(&pdev->dev), fimc);
+	ret = devm_request_irq(dev, res->start, fimc_irq_handler,
+			       0, dev_name(dev), fimc);
 	if (ret) {
-		dev_err(&pdev->dev, "failed to install irq (%d)\n", ret);
+		dev_err(dev, "failed to install irq (%d)\n", ret);
 		goto err_clk;
 	}
 
@@ -939,23 +950,23 @@ static int fimc_probe(struct platform_device *pdev)
 		goto err_clk;
 
 	platform_set_drvdata(pdev, fimc);
-	pm_runtime_enable(&pdev->dev);
-	ret = pm_runtime_get_sync(&pdev->dev);
+	pm_runtime_enable(dev);
+	ret = pm_runtime_get_sync(dev);
 	if (ret < 0)
 		goto err_sd;
 	/* Initialize contiguous memory allocator */
-	fimc->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
+	fimc->alloc_ctx = vb2_dma_contig_init_ctx(dev);
 	if (IS_ERR(fimc->alloc_ctx)) {
 		ret = PTR_ERR(fimc->alloc_ctx);
 		goto err_pm;
 	}
 
-	dev_dbg(&pdev->dev, "FIMC.%d registered successfully\n", fimc->id);
+	dev_dbg(dev, "FIMC.%d registered successfully\n", fimc->id);
 
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_put(dev);
 	return 0;
 err_pm:
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_put(dev);
 err_sd:
 	fimc_unregister_capture_subdev(fimc);
 err_clk:
@@ -1267,10 +1278,24 @@ static const struct platform_device_id fimc_driver_ids[] = {
 		.name		= "exynos4x12-fimc",
 		.driver_data	= (unsigned long)&fimc_drvdata_exynos4x12,
 	},
-	{},
+	{ },
 };
 MODULE_DEVICE_TABLE(platform, fimc_driver_ids);
 
+static const struct of_device_id fimc_of_match[] __devinitconst = {
+	{
+		.compatible = "samsung,s5pv210-fimc",
+		.data = &fimc_drvdata_s5pv210,
+	}, {
+		.compatible = "samsung,exynos4210-fimc",
+		.data = &fimc_drvdata_exynos4210,
+	}, {
+		.compatible = "samsung,exynos4212-fimc",
+		.data = &fimc_drvdata_exynos4x12,
+	},
+	{ /* sentinel */ },
+};
+
 static const struct dev_pm_ops fimc_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(fimc_suspend, fimc_resume)
 	SET_RUNTIME_PM_OPS(fimc_runtime_suspend, fimc_runtime_resume, NULL)
@@ -1281,9 +1306,10 @@ static struct platform_driver fimc_driver = {
 	.remove		= __devexit_p(fimc_remove),
 	.id_table	= fimc_driver_ids,
 	.driver = {
-		.name	= FIMC_MODULE_NAME,
-		.owner	= THIS_MODULE,
-		.pm     = &fimc_pm_ops,
+		.of_match_table = of_match_ptr(fimc_of_match),
+		.name		= FIMC_MODULE_NAME,
+		.owner		= THIS_MODULE,
+		.pm     	= &fimc_pm_ops,
 	}
 };
 
-- 
1.7.9.5

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

* [PATCH RFC v2 05/15] s5p-fimc: Support for FIMC-LITE devices instantiated from the device tree
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
                   ` (3 preceding siblings ...)
  2012-12-31 16:03 ` [PATCH RFC v2 04/15] s5p-fimc: Support for FIMC devices instantiated from the device tree Sylwester Nawrocki
@ 2012-12-31 16:03 ` Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 06/15] s5p-fimc: Change platform subdevs registration method Sylwester Nawrocki
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

This patch add support for binding the driver to FIMC-LITE devices
instantiated from the device tree.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 .../devicetree/bindings/media/soc/samsung-fimc.txt |   16 +++++
 drivers/media/platform/s5p-fimc/fimc-lite.c        |   65 ++++++++++++++------
 2 files changed, 62 insertions(+), 19 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt
index fab7e61..5bbda07 100644
--- a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt
+++ b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt
@@ -57,6 +57,22 @@ Optional properties
 			   0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT;
 
 
+'fimc-lite' device nodes
+-----------------------
+
+Required properties:
+
+- compatible : should be "samsung,exynos4212-fimc" for Exynos4212 and
+	       Exynos4412 SoCs;
+- reg	     : physical base address and size of the device memory mapped
+	       registers;
+- interrupts : should contain FIMC-LITE interrupt;
+
+For every fimc-lite node a numbered alias should be present in the aliases
+node. Aliases are in form of fimc-lite<n>, where <n> is an integer (0...N)
+specifying the IP's instance index.
+
+
 Example:
 
 	aliases {
diff --git a/drivers/media/platform/s5p-fimc/fimc-lite.c b/drivers/media/platform/s5p-fimc/fimc-lite.c
index ef31c39..cfa3952 100644
--- a/drivers/media/platform/s5p-fimc/fimc-lite.c
+++ b/drivers/media/platform/s5p-fimc/fimc-lite.c
@@ -17,6 +17,7 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/types.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -1490,18 +1491,34 @@ static int fimc_lite_clk_get(struct fimc_lite *fimc)
 	return ret;
 }
 
+static const struct of_device_id flite_of_match[];
+
 static int __devinit fimc_lite_probe(struct platform_device *pdev)
 {
-	struct flite_drvdata *drv_data = fimc_lite_get_drvdata(pdev);
+	struct flite_drvdata *drv_data = NULL;
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *of_id;
 	struct fimc_lite *fimc;
 	struct resource *res;
 	int ret;
 
-	fimc = devm_kzalloc(&pdev->dev, sizeof(*fimc), GFP_KERNEL);
+	fimc = devm_kzalloc(dev, sizeof(*fimc), GFP_KERNEL);
 	if (!fimc)
 		return -ENOMEM;
 
-	fimc->index = pdev->id;
+	if (dev->of_node) {
+		of_id = of_match_node(flite_of_match, dev->of_node);
+		if (of_id)
+			drv_data = (struct flite_drvdata *)of_id->data;
+		fimc->index = of_alias_get_id(dev->of_node, "fimc-lite");
+	} else {
+		drv_data = fimc_lite_get_drvdata(pdev);
+		fimc->index = pdev->id;
+	}
+
+	if (!drv_data || fimc->index < 0 || fimc->index >= FIMC_LITE_MAX_DEVS)
+		return -EINVAL;
+
 	fimc->variant = drv_data->variant[fimc->index];
 	fimc->pdev = pdev;
 
@@ -1510,15 +1527,15 @@ static int __devinit fimc_lite_probe(struct platform_device *pdev)
 	mutex_init(&fimc->lock);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	fimc->regs = devm_request_and_ioremap(&pdev->dev, res);
+	fimc->regs = devm_request_and_ioremap(dev, res);
 	if (fimc->regs == NULL) {
-		dev_err(&pdev->dev, "Failed to obtain io memory\n");
+		dev_err(dev, "Failed to obtain io memory\n");
 		return -ENOENT;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (res == NULL) {
-		dev_err(&pdev->dev, "Failed to get IRQ resource\n");
+		dev_err(dev, "Failed to get IRQ resource\n");
 		return -ENXIO;
 	}
 
@@ -1526,10 +1543,10 @@ static int __devinit fimc_lite_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = devm_request_irq(&pdev->dev, res->start, flite_irq_handler,
-			       0, dev_name(&pdev->dev), fimc);
+	ret = devm_request_irq(dev, res->start, flite_irq_handler,
+			       0, dev_name(dev), fimc);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to install irq (%d)\n", ret);
+		dev_err(dev, "Failed to install irq (%d)\n", ret);
 		goto err_clk;
 	}
 
@@ -1539,23 +1556,23 @@ static int __devinit fimc_lite_probe(struct platform_device *pdev)
 		goto err_clk;
 
 	platform_set_drvdata(pdev, fimc);
-	pm_runtime_enable(&pdev->dev);
-	ret = pm_runtime_get_sync(&pdev->dev);
+	pm_runtime_enable(dev);
+	ret = pm_runtime_get_sync(dev);
 	if (ret < 0)
 		goto err_sd;
 
-	fimc->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
+	fimc->alloc_ctx = vb2_dma_contig_init_ctx(dev);
 	if (IS_ERR(fimc->alloc_ctx)) {
 		ret = PTR_ERR(fimc->alloc_ctx);
 		goto err_pm;
 	}
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_put(dev);
 
-	dev_dbg(&pdev->dev, "FIMC-LITE.%d registered successfully\n",
+	dev_dbg(dev, "FIMC-LITE.%d registered successfully\n",
 		fimc->index);
 	return 0;
 err_pm:
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_put(dev);
 err_sd:
 	fimc_lite_unregister_capture_subdev(fimc);
 err_clk:
@@ -1646,6 +1663,12 @@ static int __devexit fimc_lite_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct dev_pm_ops fimc_lite_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(fimc_lite_suspend, fimc_lite_resume)
+	SET_RUNTIME_PM_OPS(fimc_lite_runtime_suspend, fimc_lite_runtime_resume,
+			   NULL)
+};
+
 static struct flite_variant fimc_lite0_variant_exynos4 = {
 	.max_width		= 8192,
 	.max_height		= 8192,
@@ -1671,17 +1694,21 @@ static struct platform_device_id fimc_lite_driver_ids[] = {
 };
 MODULE_DEVICE_TABLE(platform, fimc_lite_driver_ids);
 
-static const struct dev_pm_ops fimc_lite_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(fimc_lite_suspend, fimc_lite_resume)
-	SET_RUNTIME_PM_OPS(fimc_lite_runtime_suspend, fimc_lite_runtime_resume,
-			   NULL)
+static const struct of_device_id flite_of_match[] __devinitconst = {
+	{
+		.compatible = "samsung,exynos4212-fimc-lite",
+		.data = &fimc_lite_drvdata_exynos4,
+	},
+	{ /* sentinel */ },
 };
+MODULE_DEVICE_TABLE(of, flite_of_match);
 
 static struct platform_driver fimc_lite_driver = {
 	.probe		= fimc_lite_probe,
 	.remove		= __devexit_p(fimc_lite_remove),
 	.id_table	= fimc_lite_driver_ids,
 	.driver = {
+		.of_match_table = of_match_ptr(flite_of_match),
 		.name		= FIMC_LITE_DRV_NAME,
 		.owner		= THIS_MODULE,
 		.pm		= &fimc_lite_pm_ops,
-- 
1.7.9.5

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

* [PATCH RFC v2 06/15] s5p-fimc: Change platform subdevs registration method
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
                   ` (4 preceding siblings ...)
  2012-12-31 16:03 ` [PATCH RFC v2 05/15] s5p-fimc: Support for FIMC-LITE " Sylwester Nawrocki
@ 2012-12-31 16:03 ` Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 07/15] s5p-fimc: Support camera media device initialization on DT systems Sylwester Nawrocki
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

The previous method of registering platform entities into the main
driver using driver_find() and then iterating over devices bound to
a driver was racy and is being removed here. Nothing was preventing
module from unloading during a call to try_module_get(driver->owner).
Instead, we look up a device first and then check for its driver while
holding device lock.

The platform sub-devices are looked up and registered to the top
level driver. When any sub-device is not yet initialized and ready
the main driver's probe() will be deferred.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-fimc/fimc-mdevice.c |  206 +++++++++++++-----------
 1 file changed, 109 insertions(+), 97 deletions(-)

diff --git a/drivers/media/platform/s5p-fimc/fimc-mdevice.c b/drivers/media/platform/s5p-fimc/fimc-mdevice.c
index 80d8fd1..bb73d17 100644
--- a/drivers/media/platform/s5p-fimc/fimc-mdevice.c
+++ b/drivers/media/platform/s5p-fimc/fimc-mdevice.c
@@ -1,8 +1,8 @@
 /*
  * S5P/EXYNOS4 SoC series camera host interface media device driver
  *
- * Copyright (C) 2011 Samsung Electronics Co., Ltd.
- * Contact: Sylwester Nawrocki, <s.nawrocki@samsung.com>
+ * Copyright (C) 2011 - 2012 Samsung Electronics Co., Ltd.
+ * Sylwester Nawrocki <s.nawrocki@samsung.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published
@@ -312,138 +312,148 @@ static int fimc_md_register_sensor_entities(struct fimc_md *fmd)
 }
 
 /*
- * MIPI CSIS and FIMC platform devices registration.
+ * MIPI-CSIS, FIMC and FIMC-LITE platform devices registration.
  */
-static int fimc_register_callback(struct device *dev, void *p)
+
+static int register_fimc_lite_entity(struct fimc_md *fmd,
+				     struct fimc_lite *fimc_lite)
 {
-	struct fimc_dev *fimc = dev_get_drvdata(dev);
 	struct v4l2_subdev *sd;
-	struct fimc_md *fmd = p;
 	int ret;
 
-	if (fimc == NULL || fimc->id >= FIMC_MAX_DEVS)
-		return 0;
+	if (WARN_ON(fimc_lite->index >= FIMC_LITE_MAX_DEVS ||
+		    fmd->fimc_lite[fimc_lite->index]))
+		return -EBUSY;
 
-	sd = &fimc->vid_cap.subdev;
-	sd->grp_id = GRP_ID_FIMC;
+	sd = &fimc_lite->subdev;
+	sd->grp_id = GRP_ID_FLITE;
 	v4l2_set_subdev_hostdata(sd, (void *)&fimc_pipeline_ops);
 
 	ret = v4l2_device_register_subdev(&fmd->v4l2_dev, sd);
-	if (ret) {
-		v4l2_err(&fmd->v4l2_dev, "Failed to register FIMC.%d (%d)\n",
-			 fimc->id, ret);
-		return ret;
-	}
-
-	fmd->fimc[fimc->id] = fimc;
-	return 0;
+	if (!ret)
+		fmd->fimc_lite[fimc_lite->index] = fimc_lite;
+	else
+		v4l2_err(&fmd->v4l2_dev, "Failed to register FIMC.LITE%d\n",
+			 fimc_lite->index);
+	return ret;
 }
 
-static int fimc_lite_register_callback(struct device *dev, void *p)
+static int register_fimc_entity(struct fimc_md *fmd, struct fimc_dev *fimc)
 {
-	struct fimc_lite *fimc = dev_get_drvdata(dev);
-	struct fimc_md *fmd = p;
+	struct v4l2_subdev *sd;
 	int ret;
 
-	if (fimc == NULL || fimc->index >= FIMC_LITE_MAX_DEVS)
-		return 0;
-
-	fimc->subdev.grp_id = GRP_ID_FLITE;
-	v4l2_set_subdev_hostdata(&fimc->subdev, (void *)&fimc_pipeline_ops);
+	if (WARN_ON(fimc->id >= FIMC_MAX_DEVS || fmd->fimc[fimc->id]))
+		return -EBUSY;
 
-	ret = v4l2_device_register_subdev(&fmd->v4l2_dev, &fimc->subdev);
-	if (ret) {
-		v4l2_err(&fmd->v4l2_dev,
-			 "Failed to register FIMC-LITE.%d (%d)\n",
-			 fimc->index, ret);
-		return ret;
-	}
+	sd = &fimc->vid_cap.subdev;
+	sd->grp_id = GRP_ID_FIMC;
+	v4l2_set_subdev_hostdata(sd, (void *)&fimc_pipeline_ops);
 
-	fmd->fimc_lite[fimc->index] = fimc;
-	return 0;
+	ret = v4l2_device_register_subdev(&fmd->v4l2_dev, sd);
+	if (!ret)
+		fmd->fimc[fimc->id] = fimc;
+	else
+		v4l2_err(&fmd->v4l2_dev, "Failed to register FIMC.%d (%d)\n",
+			 fimc->id, ret);
+	return ret;
 }
 
-static int csis_register_callback(struct device *dev, void *p)
+static int register_csis_entity(struct fimc_md *fmd,
+				struct platform_device *pdev,
+				struct v4l2_subdev *sd)
 {
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct platform_device *pdev;
-	struct fimc_md *fmd = p;
-	int id, ret;
+	struct device_node *node = pdev->dev.of_node;
+	int id = 0;
+	int ret;
 
-	if (!sd)
-		return 0;
-	pdev = v4l2_get_subdevdata(sd);
-	if (!pdev || pdev->id < 0 || pdev->id >= CSIS_MAX_ENTITIES)
+	if (WARN_ON(id >= CSIS_MAX_ENTITIES || fmd->csis[id].sd))
+		return -EBUSY;
+
+	id = node ? of_alias_get_id(node, "csis") : max(0, pdev->id);
+
+	if (WARN_ON(id >= CSIS_MAX_ENTITIES))
 		return 0;
-	v4l2_info(sd, "csis%d sd: %s\n", pdev->id, sd->name);
 
-	id = pdev->id < 0 ? 0 : pdev->id;
 	sd->grp_id = GRP_ID_CSIS;
-
 	ret = v4l2_device_register_subdev(&fmd->v4l2_dev, sd);
 	if (!ret)
 		fmd->csis[id].sd = sd;
 	else
 		v4l2_err(&fmd->v4l2_dev,
-			 "Failed to register CSIS subdevice: %d\n", ret);
+			 "Failed to register MIPI-CSIS.%d (%d)\n", id, ret);
 	return ret;
 }
 
-/**
- * fimc_md_register_platform_entities - register FIMC and CSIS media entities
- */
-static int fimc_md_register_platform_entities(struct fimc_md *fmd)
+static int fimc_md_register_platform_entity(struct fimc_md *fmd,
+					    struct platform_device *pdev,
+					    int plat_entity)
 {
-	struct s5p_platform_fimc *pdata = fmd->pdev->dev.platform_data;
-	struct device_driver *driver;
-	int ret, i;
-
-	driver = driver_find(FIMC_MODULE_NAME, &platform_bus_type);
-	if (!driver) {
-		v4l2_warn(&fmd->v4l2_dev,
-			 "%s driver not found, deffering probe\n",
-			 FIMC_MODULE_NAME);
-		return -EPROBE_DEFER;
-	}
-
-	ret = driver_for_each_device(driver, NULL, fmd,
-				     fimc_register_callback);
-	if (ret)
-		return ret;
-
-	driver = driver_find(FIMC_LITE_DRV_NAME, &platform_bus_type);
-	if (driver && try_module_get(driver->owner)) {
-		ret = driver_for_each_device(driver, NULL, fmd,
-					     fimc_lite_register_callback);
-		if (ret)
-			return ret;
-		module_put(driver->owner);
-	}
-	/*
-	 * Check if there is any sensor on the MIPI-CSI2 bus and
-	 * if not skip the s5p-csis module loading.
-	 */
-	if (pdata == NULL)
-		return 0;
-	for (i = 0; i < pdata->num_clients; i++) {
-		if (pdata->isp_info[i].bus_type == FIMC_MIPI_CSI2) {
-			ret = 1;
+	struct device *dev = &pdev->dev;
+	int ret = -EPROBE_DEFER;
+	void *drvdata;
+
+	/* Lock to ensure dev->driver won't change. */
+	device_lock(dev);
+
+	if (!dev->driver || !try_module_get(dev->driver->owner))
+		goto dev_unlock;
+
+	drvdata = dev_get_drvdata(dev);
+	/* Some subdev didn't probe succesfully id drvdata is NULL */
+	if (drvdata) {
+		switch (plat_entity) {
+		case IDX_FIMC:
+			ret = register_fimc_entity(fmd, drvdata);
 			break;
+		case IDX_FLITE:
+			ret = register_fimc_lite_entity(fmd, drvdata);
+			break;
+		case IDX_CSIS:
+			ret = register_csis_entity(fmd, pdev, drvdata);
+			break;
+		default:
+			ret = -ENODEV;
 		}
 	}
-	if (!ret)
-		return 0;
 
-	driver = driver_find(CSIS_DRIVER_NAME, &platform_bus_type);
-	if (!driver || !try_module_get(driver->owner)) {
-		v4l2_warn(&fmd->v4l2_dev,
-			 "%s driver not found, deffering probe\n",
-			 CSIS_DRIVER_NAME);
-		return -EPROBE_DEFER;
+	module_put(dev->driver->owner);
+dev_unlock:
+	device_unlock(dev);
+	if (ret == -EPROBE_DEFER)
+		dev_info(&fmd->pdev->dev, "deferring %s device registration\n",
+			dev_name(dev));
+	else if (ret < 0)
+		dev_err(&fmd->pdev->dev, "%s device registration failed (%d)\n",
+			dev_name(dev), ret);
+	return ret;
+}
+
+static int fimc_md_pdev_match(struct device *dev, void *data)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	int plat_entity = -1;
+	int ret;
+	char *p;
+
+	if (!get_device(dev))
+		return -ENODEV;
+
+	if (!strcmp(pdev->name, CSIS_DRIVER_NAME)) {
+		plat_entity = IDX_CSIS;
+	} else if (!strcmp(pdev->name, FIMC_LITE_DRV_NAME)) {
+		plat_entity = IDX_FLITE;
+	} else {
+		p = strstr(pdev->name, "fimc");
+		if (p && *(p + 4) == 0)
+			plat_entity = IDX_FIMC;
 	}
 
-	return driver_for_each_device(driver, NULL, fmd,
-				      csis_register_callback);
+	if (plat_entity >= 0)
+		ret = fimc_md_register_platform_entity(data, pdev,
+						       plat_entity);
+	put_device(dev);
+	return 0;
 }
 
 static void fimc_md_unregister_entities(struct fimc_md *fmd)
@@ -477,6 +487,7 @@ static void fimc_md_unregister_entities(struct fimc_md *fmd)
 		fimc_md_unregister_sensor(fmd->sensor[i].subdev);
 		fmd->sensor[i].subdev = NULL;
 	}
+	v4l2_info(&fmd->v4l2_dev, "Unregistered all entities\n");
 }
 
 /**
@@ -939,7 +950,8 @@ static int fimc_md_probe(struct platform_device *pdev)
 	/* Protect the media graph while we're registering entities */
 	mutex_lock(&fmd->media_dev.graph_mutex);
 
-	ret = fimc_md_register_platform_entities(fmd);
+	ret = bus_for_each_dev(&platform_bus_type, NULL, fmd,
+					fimc_md_pdev_match);
 	if (ret)
 		goto err_unlock;
 
-- 
1.7.9.5

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

* [PATCH RFC v2 07/15] s5p-fimc: Support camera media device initialization on DT systems
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
                   ` (5 preceding siblings ...)
  2012-12-31 16:03 ` [PATCH RFC v2 06/15] s5p-fimc: Change platform subdevs registration method Sylwester Nawrocki
@ 2012-12-31 16:03 ` Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 08/15] s5p-fimc: Add device tree based sensors registration Sylwester Nawrocki
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

This patch add changes required for the main camera media device
driver to be initialized on systems booted from the device tree.

The platform devices corresponding to child dt nodes of the 'camera'
node are looked up and and registered as sub-devices to the common
media device. The main driver's probing is deferred if any of the
sub-device drivers is not yet initialized and ready.

An OF matching table is added for the main driver associated with
the 'camera' node.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-fimc/fimc-core.c    |    1 -
 drivers/media/platform/s5p-fimc/fimc-mdevice.c |   67 +++++++++++++++++++++---
 drivers/media/platform/s5p-fimc/fimc-mdevice.h |    4 ++
 3 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/s5p-fimc/fimc-core.c b/drivers/media/platform/s5p-fimc/fimc-core.c
index e7eabb7..b02edac 100644
--- a/drivers/media/platform/s5p-fimc/fimc-core.c
+++ b/drivers/media/platform/s5p-fimc/fimc-core.c
@@ -1280,7 +1280,6 @@ static const struct platform_device_id fimc_driver_ids[] = {
 	},
 	{ },
 };
-MODULE_DEVICE_TABLE(platform, fimc_driver_ids);
 
 static const struct of_device_id fimc_of_match[] __devinitconst = {
 	{
diff --git a/drivers/media/platform/s5p-fimc/fimc-mdevice.c b/drivers/media/platform/s5p-fimc/fimc-mdevice.c
index bb73d17..105bb91 100644
--- a/drivers/media/platform/s5p-fimc/fimc-mdevice.c
+++ b/drivers/media/platform/s5p-fimc/fimc-mdevice.c
@@ -17,6 +17,8 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/types.h>
@@ -456,6 +458,43 @@ static int fimc_md_pdev_match(struct device *dev, void *data)
 	return 0;
 }
 
+/* Register FIMC, FIMC-LITE and CSIS media entities */
+#ifdef CONFIG_OF
+static int fimc_md_register_of_platform_entities(struct fimc_md *fmd)
+{
+	struct device_node *node;
+	int ret = 0;
+
+	for_each_available_child_of_node(fmd->pdev->dev.of_node, node) {
+		struct platform_device *pdev;
+		int plat_entity = -1;
+
+		pdev = of_find_device_by_node(node);
+		if (!pdev)
+			return -ENODEV;
+
+		/* If driver of any entity isn't ready try all again later. */
+		if (!strcmp(node->name, CSIS_OF_NODE_NAME))
+			plat_entity = IDX_CSIS;
+		else if (!strcmp(node->name, FIMC_LITE_OF_NODE_NAME))
+			plat_entity = IDX_FLITE;
+		else if	(!strcmp(node->name, FIMC_OF_NODE_NAME))
+			plat_entity = IDX_FIMC;
+
+		if (plat_entity >= 0)
+			ret = fimc_md_register_platform_entity(fmd, pdev,
+							plat_entity);
+		put_device(&pdev->dev);
+		if (ret < 0)
+			break;
+	}
+
+	return ret;
+}
+#else
+#define fimc_md_register_platform_entities(fmd) (-ENOSYS)
+#endif
+
 static void fimc_md_unregister_entities(struct fimc_md *fmd)
 {
 	int i;
@@ -928,8 +967,8 @@ static int fimc_md_probe(struct platform_device *pdev)
 	v4l2_dev = &fmd->v4l2_dev;
 	v4l2_dev->mdev = &fmd->media_dev;
 	v4l2_dev->notify = fimc_sensor_notify;
-	snprintf(v4l2_dev->name, sizeof(v4l2_dev->name), "%s",
-		 dev_name(&pdev->dev));
+	strlcpy(v4l2_dev->name, "s5p-fimc-md", sizeof(v4l2_dev->name));
+
 
 	ret = v4l2_device_register(&pdev->dev, &fmd->v4l2_dev);
 	if (ret < 0) {
@@ -950,8 +989,11 @@ static int fimc_md_probe(struct platform_device *pdev)
 	/* Protect the media graph while we're registering entities */
 	mutex_lock(&fmd->media_dev.graph_mutex);
 
-	ret = bus_for_each_dev(&platform_bus_type, NULL, fmd,
-					fimc_md_pdev_match);
+	if (fmd->pdev->dev.of_node)
+		ret = fimc_md_register_of_platform_entities(fmd);
+	else
+		ret = bus_for_each_dev(&platform_bus_type, NULL, fmd,
+						fimc_md_pdev_match);
 	if (ret)
 		goto err_unlock;
 
@@ -999,12 +1041,25 @@ static int __devexit fimc_md_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct platform_device_id fimc_driver_ids[] __always_unused = {
+	{ .name = "s5p-fimc-md" },
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, fimc_driver_ids);
+
+static const struct of_device_id fimc_md_of_match[] __initconst = {
+	{ .compatible = "samsung,fimc" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, fimc_md_of_match);
+
 static struct platform_driver fimc_md_driver = {
 	.probe		= fimc_md_probe,
 	.remove		= __devexit_p(fimc_md_remove),
 	.driver = {
-		.name	= "s5p-fimc-md",
-		.owner	= THIS_MODULE,
+		.of_match_table = fimc_md_of_match,
+		.name		= "s5p-fimc-md",
+		.owner		= THIS_MODULE,
 	}
 };
 
diff --git a/drivers/media/platform/s5p-fimc/fimc-mdevice.h b/drivers/media/platform/s5p-fimc/fimc-mdevice.h
index da7d992..1b7850c 100644
--- a/drivers/media/platform/s5p-fimc/fimc-mdevice.h
+++ b/drivers/media/platform/s5p-fimc/fimc-mdevice.h
@@ -21,6 +21,10 @@
 #include "fimc-lite.h"
 #include "mipi-csis.h"
 
+#define FIMC_OF_NODE_NAME	"fimc"
+#define FIMC_LITE_OF_NODE_NAME	"fimc_lite"
+#define CSIS_OF_NODE_NAME	"csis"
+
 /* Group IDs of sensor, MIPI-CSIS, FIMC-LITE and the writeback subdevs. */
 #define GRP_ID_SENSOR		(1 << 8)
 #define GRP_ID_FIMC_IS_SENSOR	(1 << 9)
-- 
1.7.9.5

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

* [PATCH RFC v2 08/15] s5p-fimc: Add device tree based sensors registration
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
                   ` (6 preceding siblings ...)
  2012-12-31 16:03 ` [PATCH RFC v2 07/15] s5p-fimc: Support camera media device initialization on DT systems Sylwester Nawrocki
@ 2012-12-31 16:03 ` Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 09/15] s5p-fimc: Use pinctrl API for camera ports configuration Sylwester Nawrocki
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

The sensor (I2C and/or SPI client) devices are instantiated by their
corresponding control bus drivers. Since the I2C client's master clock
is often provided by a video bus receiver (host interface) or other
than I2C/SPI controller device, the drivers of those client devices
are not accessing hardware in their driver's probe() callback. Instead,
after enabling clock, the host driver calls back into a sub-device
when it wants to activate them. This pattern is used by some in-tree
drivers and this patch also uses it for DT case. This patch is
intended as a first step for adding device tree support to the
S5P/Exynos SoC camera drivers. The second one is adding support for
asynchronous sub-devices registration and clock control from
sub-device driver level. The bindings shall not change when
asynchronous probing support is added. The motivation behind this
approach is to have basic support for device tree based platforms
in the driver, while asynchronous subdev probing and related issues
are being discussed on LMML.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 .../devicetree/bindings/media/soc/samsung-fimc.txt |   75 +++++++
 drivers/media/platform/s5p-fimc/fimc-mdevice.c     |  237 +++++++++++++++++---
 include/media/s5p_fimc.h                           |   16 ++
 3 files changed, 299 insertions(+), 29 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt
index 5bbda07..82bd619 100644
--- a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt
+++ b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt
@@ -73,6 +73,15 @@ node. Aliases are in form of fimc-lite<n>, where <n> is an integer (0...N)
 specifying the IP's instance index.
 
 
+Image sensor nodes
+------------------
+
+The sensor device nodes should be added as their control bus controller
+(e.g. I2C0) child nodes and linked to a port created under csis or
+parallel-ports node, using common bindings for video input interfaces,
+.i.e. port/endpoint node pairs. The implementation of this binding requires
+at clock-frequency property to be present under sensor device nodes.
+
 Example:
 
 	aliases {
@@ -80,6 +89,47 @@ Example:
 		fimc0 = &fimc_0;
 	};
 
+	/* Parallel bus IF sensor */
+	i2c_0: i2c@13860000 {
+		s5k6aa: sensor@3c {
+			compatible = "samsung,s5k6aafx";
+			reg = <0x3c>;
+			vddio-supply = <...>;
+
+			clock-frequency = <24000000>;
+			clocks = <...>;
+			clock-names = "mclk";
+
+			port {
+				s5k6aa_ep: endpoint {
+					remote-endpoint = <&fimc0_ep>;
+					bus-width = <8>;
+					hsync-active = <0>;
+					hsync-active = <1>;
+					pclk-sample = <1>;
+				};
+			};
+		};
+	};
+
+	/* MIPI CSI-2 bus IF sensor */
+	s5c73m3: sensor@0x1a {
+		compatible = "samsung,s5c73m3";
+		reg = <0x1a>;
+		vddio-supply = <...>;
+
+		clock-frequency = <24000000>;
+		clocks = <...>;
+		clock-names = "mclk";
+
+		port {
+			s5c73m3_1: endpoint {
+				data-lanes = <1>, <2>, <3>, <4>;
+				remote-endpoint = <&csis0_ep>;
+			};
+		};
+	};
+
 	camera {
 		compatible = "samsung,fimc", "simple-bus";
 		#address-cells = <1>;
@@ -90,6 +140,21 @@ Example:
 		pinctrl-0 = <&cam_port_a_clk_active>;
 		pinctrl-1 = <&cam_port_a_clk_idle>;
 
+		/* parallel camera ports */
+		parallel-ports {
+			/* camera A input */
+			port@0 {
+				reg = <0>;
+				fimc0_ep: endpoint {
+					remote-endpoint = <&s5k6aa_ep>;
+					bus-width = <8>;
+					hsync-active = <0>;
+					hsync-active = <1>;
+					pclk-sample = <1>;
+				};
+			};
+		};
+
 		fimc_0: fimc@11800000 {
 			compatible = "samsung,exynos4210-fimc";
 			reg = <0x11800000 0x1000>;
@@ -102,6 +167,16 @@ Example:
 			reg = <0x11880000 0x1000>;
 			interrupts = <0 78 0>;
 			max-data-lanes = <4>;
+			/* camera C input */
+			port {
+				reg = <2>;
+				csis0_ep: endpoint {
+					remote-endpoint = <&s5c73m3_ep>;
+					data-lanes = <1>, <2>, <3>, <4>;
+					samsung,csis-hs-settle = <12>;
+					samsung,camclk-out = <0>;
+				};
+			};
 		};
 	};
 
diff --git a/drivers/media/platform/s5p-fimc/fimc-mdevice.c b/drivers/media/platform/s5p-fimc/fimc-mdevice.c
index 105bb91..3ac6ea8 100644
--- a/drivers/media/platform/s5p-fimc/fimc-mdevice.c
+++ b/drivers/media/platform/s5p-fimc/fimc-mdevice.c
@@ -19,11 +19,15 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/of_i2c.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/types.h>
 #include <linux/slab.h>
 #include <media/v4l2-ctrls.h>
+#include <media/v4l2-of.h>
 #include <media/media-device.h>
 #include <media/s5p_fimc.h>
 
@@ -248,7 +252,7 @@ static struct v4l2_subdev *fimc_md_register_sensor(struct fimc_md *fmd,
 	sd->grp_id = GRP_ID_SENSOR;
 
 	v4l2_info(&fmd->v4l2_dev, "Registered sensor subdevice %s\n",
-		  s_info->pdata.board_info->type);
+		  sd->name);
 	return sd;
 }
 
@@ -260,17 +264,183 @@ static void fimc_md_unregister_sensor(struct v4l2_subdev *sd)
 	if (!client)
 		return;
 	v4l2_device_unregister_subdev(sd);
-	adapter = client->adapter;
-	i2c_unregister_device(client);
-	if (adapter)
-		i2c_put_adapter(adapter);
+
+	if (!client->dev.of_node) {
+		adapter = client->adapter;
+		i2c_unregister_device(client);
+		if (adapter)
+			i2c_put_adapter(adapter);
+	}
+}
+
+#ifdef CONFIG_OF
+/* Register I2C client subdev associated with @node. */
+static int fimc_md_of_add_sensor(struct fimc_md *fmd,
+				 struct device_node *node, int index)
+{
+	struct fimc_sensor_info *si;
+	struct i2c_client *client;
+	struct v4l2_subdev *sd;
+	int ret;
+
+	if (index >= ARRAY_SIZE(fmd->sensor))
+		return -EINVAL;
+	si = &fmd->sensor[index];
+
+	client = of_find_i2c_device_by_node(node);
+	if (!client)
+		return -EPROBE_DEFER;
+
+	device_lock(&client->dev);
+
+	if (!client->driver ||
+	    !try_module_get(client->driver->driver.owner)) {
+		ret = -EAGAIN;
+		goto dev_put;
+	}
+
+	/* Enable sensor's master clock */
+	ret = __fimc_md_set_camclk(fmd, si, true);
+	if (ret < 0)
+		goto mod_put;
+	sd = i2c_get_clientdata(client);
+
+	ret = v4l2_device_register_subdev(&fmd->v4l2_dev, sd);
+	__fimc_md_set_camclk(fmd, si, false);
+	if (ret < 0)
+		goto mod_put;
+
+	v4l2_set_subdev_hostdata(sd, si);
+	sd->grp_id = GRP_ID_SENSOR;
+	si->subdev = sd;
+	v4l2_info(&fmd->v4l2_dev, "Registered sensor subdevice: %s (%d)\n",
+		  sd->name, fmd->num_sensors);
+	fmd->num_sensors++;
+
+mod_put:
+	module_put(client->driver->driver.owner);
+dev_put:
+	device_unlock(&client->dev);
+	put_device(&client->dev);
+	return ret;
 }
 
+/* Parse port node and register as a sub-device any sensor specified there. */
+static int fimc_md_parse_port_node(struct fimc_md *fmd,
+				   struct device_node *port,
+				   unsigned int index)
+{
+	struct device_node *rem, *endpoint;
+	struct s5p_fimc_isp_info *pd;
+	struct v4l2_of_endpoint bus_cfg;
+	u32 tmp, reg;
+	int ret;
+
+	if (index >= FIMC_MAX_SENSORS ||
+	    of_property_read_u32(port, "reg", &reg))
+		return -EINVAL;
+
+	pd = &fmd->sensor[index].pdata;
+	pd->mux_id = (reg - 1) & 0x1;
+
+	endpoint = of_get_child_by_name(port, "endpoint");
+	if (!endpoint)
+		return -EINVAL;
+
+	rem = v4l2_of_get_remote_port_parent(endpoint);
+	of_node_put(endpoint);
+	if (!rem)
+		return -EINVAL;
+
+	if (!of_property_read_u32(rem, "samsung,camclk-out", &tmp))
+		pd->clk_id = tmp;
+
+	if (!of_property_read_u32(rem, "clock-frequency", &tmp))
+		pd->clk_frequency = tmp;
+
+	if (pd->clk_frequency == 0) {
+		v4l2_err(&fmd->v4l2_dev,
+			 "Wrong or unspecified sensor clock frequency\n");
+		of_node_put(rem);
+		return -EINVAL;
+	}
+
+	if (fimc_input_is_parallel(reg)) {
+		v4l2_of_parse_parallel_bus(endpoint, &bus_cfg);
+		if (bus_cfg.mbus_type == V4L2_MBUS_PARALLEL)
+			pd->bus_type = FIMC_ITU_601;
+		else
+			pd->bus_type = FIMC_ITU_656;
+		pd->flags = bus_cfg.mbus_flags;
+	} else if (fimc_input_is_mipi_csi(reg)) {
+		/*
+		 * MIPI CSI-2: only input mux selection
+		 * and sensor's clock frequency is needed.
+		 */
+		pd->bus_type = FIMC_MIPI_CSI2;
+	} else {
+		v4l2_err(&fmd->v4l2_dev,
+			 "Wrong reg property value (%u) at node %s/endpoint\n",
+			 reg, rem->name);
+	}
+
+	ret = fimc_md_of_add_sensor(fmd, rem, index);
+	of_node_put(rem);
+
+	return ret;
+}
+
+/* Register all SoC external sub-devices */
+static int fimc_md_of_sensors_register(struct fimc_md *fmd,
+				       struct device_node *np)
+{
+	struct device_node *parent = fmd->pdev->dev.of_node;
+	struct device_node *node, *ports;
+	int index = 0;
+	int ret;
+
+	/* Attach sensors linked to MIPI CSI-2 receivers */
+	for_each_available_child_of_node(parent, node) {
+		struct device_node *port;
+
+		if (of_node_cmp(node->name, "csis"))
+			continue;
+
+		port = of_get_child_by_name(node, "port");
+		if (!port)
+			return -EINVAL;
+
+		ret = fimc_md_parse_port_node(fmd, port, index);
+		if (ret < 0)
+			return ret;
+		index++;
+	}
+
+	/* Attach sensors listed in the parallel-ports node */
+	ports = of_get_child_by_name(parent, "parallel-ports");
+	if (!ports)
+		return 0;
+
+	for_each_child_of_node(ports, node) {
+		ret = fimc_md_parse_port_node(fmd, node, index);
+		if (ret < 0)
+			break;
+		index++;
+	}
+
+	return 0;
+}
+#else
+#define fimc_md_of_sensors_register(fmd, np) (-ENOSYS)
+#endif
+
 static int fimc_md_register_sensor_entities(struct fimc_md *fmd)
 {
 	struct s5p_platform_fimc *pdata = fmd->pdev->dev.platform_data;
+	struct device_node *of_node = fmd->pdev->dev.of_node;
 	struct fimc_dev *fd = NULL;
-	int num_clients, ret, i;
+	int num_clients = 0;
+	int ret, i;
 
 	/*
 	 * Runtime resume one of the FIMC entities to make sure
@@ -281,34 +451,41 @@ static int fimc_md_register_sensor_entities(struct fimc_md *fmd)
 			fd = fmd->fimc[i];
 	if (!fd)
 		return -ENXIO;
+
 	ret = pm_runtime_get_sync(&fd->pdev->dev);
 	if (ret < 0)
 		return ret;
 
-	WARN_ON(pdata->num_clients > ARRAY_SIZE(fmd->sensor));
-	num_clients = min_t(u32, pdata->num_clients, ARRAY_SIZE(fmd->sensor));
+	if (of_node) {
+		fmd->num_sensors = 0;
+		ret = fimc_md_of_sensors_register(fmd, of_node);
+	} else if (pdata) {
+		WARN_ON(pdata->num_clients > ARRAY_SIZE(fmd->sensor));
+		num_clients = min_t(u32, pdata->num_clients,
+				    ARRAY_SIZE(fmd->sensor));
+		fmd->num_sensors = num_clients;
 
-	fmd->num_sensors = num_clients;
-	for (i = 0; i < num_clients; i++) {
-		struct v4l2_subdev *sd;
+		for (i = 0; i < num_clients; i++) {
+			struct v4l2_subdev *sd;
 
-		fmd->sensor[i].pdata = pdata->isp_info[i];
-		ret = __fimc_md_set_camclk(fmd, &fmd->sensor[i], true);
-		if (ret)
-			break;
-		sd = fimc_md_register_sensor(fmd, &fmd->sensor[i]);
-		ret = __fimc_md_set_camclk(fmd, &fmd->sensor[i], false);
-
-		if (!IS_ERR(sd)) {
+			fmd->sensor[i].pdata = pdata->isp_info[i];
+			ret = __fimc_md_set_camclk(fmd, &fmd->sensor[i], true);
+			if (ret)
+				break;
+			sd = fimc_md_register_sensor(fmd, &fmd->sensor[i]);
+			ret = __fimc_md_set_camclk(fmd, &fmd->sensor[i], false);
+
+			if (IS_ERR(sd)) {
+				fmd->sensor[i].subdev = NULL;
+				ret = PTR_ERR(sd);
+				break;
+			}
 			fmd->sensor[i].subdev = sd;
-		} else {
-			fmd->sensor[i].subdev = NULL;
-			ret = PTR_ERR(sd);
-			break;
+			if (ret)
+				break;
 		}
-		if (ret)
-			break;
 	}
+
 	pm_runtime_put(&fd->pdev->dev);
 	return ret;
 }
@@ -948,11 +1125,12 @@ static DEVICE_ATTR(subdev_conf_mode, S_IWUSR | S_IRUGO,
 
 static int fimc_md_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct v4l2_device *v4l2_dev;
 	struct fimc_md *fmd;
 	int ret;
 
-	fmd = devm_kzalloc(&pdev->dev, sizeof(*fmd), GFP_KERNEL);
+	fmd = devm_kzalloc(dev, sizeof(*fmd), GFP_KERNEL);
 	if (!fmd)
 		return -ENOMEM;
 
@@ -962,7 +1140,7 @@ static int fimc_md_probe(struct platform_device *pdev)
 	strlcpy(fmd->media_dev.model, "SAMSUNG S5P FIMC",
 		sizeof(fmd->media_dev.model));
 	fmd->media_dev.link_notify = fimc_md_link_notify;
-	fmd->media_dev.dev = &pdev->dev;
+	fmd->media_dev.dev = dev;
 
 	v4l2_dev = &fmd->v4l2_dev;
 	v4l2_dev->mdev = &fmd->media_dev;
@@ -970,7 +1148,7 @@ static int fimc_md_probe(struct platform_device *pdev)
 	strlcpy(v4l2_dev->name, "s5p-fimc-md", sizeof(v4l2_dev->name));
 
 
-	ret = v4l2_device_register(&pdev->dev, &fmd->v4l2_dev);
+	ret = v4l2_device_register(dev, &fmd->v4l2_dev);
 	if (ret < 0) {
 		v4l2_err(v4l2_dev, "Failed to register v4l2_device: %d\n", ret);
 		return ret;
@@ -997,11 +1175,12 @@ static int fimc_md_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_unlock;
 
-	if (pdev->dev.platform_data) {
+	if (dev->platform_data || dev->of_node) {
 		ret = fimc_md_register_sensor_entities(fmd);
 		if (ret)
 			goto err_unlock;
 	}
+
 	ret = fimc_md_create_links(fmd);
 	if (ret)
 		goto err_unlock;
diff --git a/include/media/s5p_fimc.h b/include/media/s5p_fimc.h
index eaea62a..e8e03cf 100644
--- a/include/media/s5p_fimc.h
+++ b/include/media/s5p_fimc.h
@@ -14,6 +14,22 @@
 
 #include <media/media-entity.h>
 
+/*
+ * Enumeration of data inputs to the camera subsystem.
+ */
+enum fimc_input {
+	FIMC_INPUT_PARALLEL_0	= 1,
+	FIMC_INPUT_PARALLEL_1,
+	FIMC_INPUT_MIPI_CSI2_0	= 3,
+	FIMC_INPUT_MIPI_CSI2_1,
+	FIMC_INPUT_WRITEBACK_A	= 5,
+	FIMC_INPUT_WRITEBACK_B,
+	FIMC_INPUT_WRITEBACK_ISP = 5,
+};
+
+#define fimc_input_is_parallel(x) ((x) == 1 || (x) == 2)
+#define fimc_input_is_mipi_csi(x) ((x) == 3 || (x) == 4)
+
 enum cam_bus_type {
 	FIMC_ITU_601 = 1,
 	FIMC_ITU_656,
-- 
1.7.9.5

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

* [PATCH RFC v2 09/15] s5p-fimc: Use pinctrl API for camera ports configuration
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
                   ` (7 preceding siblings ...)
  2012-12-31 16:03 ` [PATCH RFC v2 08/15] s5p-fimc: Add device tree based sensors registration Sylwester Nawrocki
@ 2012-12-31 16:03 ` Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 10/15] ARM: EXYNOS4: Add OF_DEV_AUXDATA for FIMC, FIMC-LITE and CSIS Sylwester Nawrocki
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

Before the camera ports can be used the pinmux needs to be configured
properly. This patch adds a function to get the pinctrl states and to
set default camera port pinmux state during the media driver's probe().
The camera port(s) are configured for video bus operation in this way.

"inactive" pinctrl state is intended for setting clock output pin(s)
into high impedance state when camera sensors are powered off.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-fimc/fimc-mdevice.c |   25 ++++++++++++++++++++++++
 drivers/media/platform/s5p-fimc/fimc-mdevice.h |    6 ++++++
 2 files changed, 31 insertions(+)

diff --git a/drivers/media/platform/s5p-fimc/fimc-mdevice.c b/drivers/media/platform/s5p-fimc/fimc-mdevice.c
index 3ac6ea8..9e4ed9e 100644
--- a/drivers/media/platform/s5p-fimc/fimc-mdevice.c
+++ b/drivers/media/platform/s5p-fimc/fimc-mdevice.c
@@ -1123,6 +1123,25 @@ static ssize_t fimc_md_sysfs_store(struct device *dev,
 static DEVICE_ATTR(subdev_conf_mode, S_IWUSR | S_IRUGO,
 		   fimc_md_sysfs_show, fimc_md_sysfs_store);
 
+static int fimc_md_get_pinctrl(struct fimc_md *fmd)
+{
+	fmd->pinctl = devm_pinctrl_get_select_default(&fmd->pdev->dev);
+	if (IS_ERR(fmd->pinctl))
+		return PTR_ERR(fmd->pinctl);
+
+	fmd->pinctl_state_default = pinctrl_lookup_state(fmd->pinctl,
+						 PINCTRL_STATE_DEFAULT);
+	if (IS_ERR(fmd->pinctl_state_default))
+		return PTR_ERR(fmd->pinctl_state_default);
+
+	fmd->pinctl_state_idle = pinctrl_lookup_state(fmd->pinctl,
+						PINCTRL_STATE_INACTIVE);
+	if (IS_ERR(fmd->pinctl_state_idle))
+		return PTR_ERR(fmd->pinctl_state_idle);
+
+	return 0;
+}
+
 static int fimc_md_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1167,6 +1186,12 @@ static int fimc_md_probe(struct platform_device *pdev)
 	/* Protect the media graph while we're registering entities */
 	mutex_lock(&fmd->media_dev.graph_mutex);
 
+	if (dev->of_node) {
+		ret = fimc_md_get_pinctrl(fmd);
+		if (ret < 0)
+			goto err_unlock;
+	}
+
 	if (fmd->pdev->dev.of_node)
 		ret = fimc_md_register_of_platform_entities(fmd);
 	else
diff --git a/drivers/media/platform/s5p-fimc/fimc-mdevice.h b/drivers/media/platform/s5p-fimc/fimc-mdevice.h
index 1b7850c..89cecaa 100644
--- a/drivers/media/platform/s5p-fimc/fimc-mdevice.h
+++ b/drivers/media/platform/s5p-fimc/fimc-mdevice.h
@@ -10,6 +10,7 @@
 #define FIMC_MDEVICE_H_
 
 #include <linux/clk.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/mutex.h>
 #include <media/media-device.h>
@@ -25,6 +26,8 @@
 #define FIMC_LITE_OF_NODE_NAME	"fimc_lite"
 #define CSIS_OF_NODE_NAME	"csis"
 
+#define PINCTRL_STATE_INACTIVE	"inactive"
+
 /* Group IDs of sensor, MIPI-CSIS, FIMC-LITE and the writeback subdevs. */
 #define GRP_ID_SENSOR		(1 << 8)
 #define GRP_ID_FIMC_IS_SENSOR	(1 << 9)
@@ -85,6 +88,9 @@ struct fimc_md {
 	struct media_device media_dev;
 	struct v4l2_device v4l2_dev;
 	struct platform_device *pdev;
+	struct pinctrl *pinctl;
+	struct pinctrl_state *pinctl_state_default;
+	struct pinctrl_state *pinctl_state_idle;
 	bool user_subdev_api;
 	spinlock_t slock;
 };
-- 
1.7.9.5

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

* [PATCH RFC v2 10/15] ARM: EXYNOS4: Add OF_DEV_AUXDATA for FIMC, FIMC-LITE and CSIS
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
                   ` (8 preceding siblings ...)
  2012-12-31 16:03 ` [PATCH RFC v2 09/15] s5p-fimc: Use pinctrl API for camera ports configuration Sylwester Nawrocki
@ 2012-12-31 16:03 ` Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 11/15] ARM: dts: Add camera node exynos4.dtsi Sylwester Nawrocki
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

Add these temporary OF_DEV_AUXDATA entries so we can use clocks
before common clock framework support for Exynos4 is available.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-exynos/mach-exynos4-dt.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/mach-exynos/mach-exynos4-dt.c b/arch/arm/mach-exynos/mach-exynos4-dt.c
index 2890bf7..3ce0c52 100644
--- a/arch/arm/mach-exynos/mach-exynos4-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos4-dt.c
@@ -77,6 +77,22 @@ static const struct of_dev_auxdata exynos4_auxdata_lookup[] __initconst = {
 				"exynos4210-spi.2", NULL),
 	OF_DEV_AUXDATA("arm,pl330", EXYNOS4_PA_PDMA0, "dma-pl330.0", NULL),
 	OF_DEV_AUXDATA("arm,pl330", EXYNOS4_PA_PDMA1, "dma-pl330.1", NULL),
+	OF_DEV_AUXDATA("samsung,exynos4210-csis", EXYNOS4_PA_MIPI_CSIS0,
+				"s5p-mipi-csis.0", NULL),
+	OF_DEV_AUXDATA("samsung,exynos4210-csis", EXYNOS4_PA_MIPI_CSIS1,
+				"s5p-mipi-csis.1", NULL),
+	OF_DEV_AUXDATA("samsung,exynos4212-fimc", EXYNOS4_PA_FIMC0,
+				"exynos4-fimc.0", NULL),
+	OF_DEV_AUXDATA("samsung,exynos4212-fimc", EXYNOS4_PA_FIMC1,
+				"exynos4-fimc.1", NULL),
+	OF_DEV_AUXDATA("samsung,exynos4212-fimc", EXYNOS4_PA_FIMC2,
+				"exynos4-fimc.2", NULL),
+	OF_DEV_AUXDATA("samsung,exynos4212-fimc", EXYNOS4_PA_FIMC3,
+				"exynos4-fimc.3", NULL),
+	OF_DEV_AUXDATA("samsung,exynos4212-fimc-lite", EXYNOS4_PA_FIMC_LITE(0),
+				"exynos-fimc-lite.0", NULL),
+	OF_DEV_AUXDATA("samsung,exynos4212-fimc-lite", EXYNOS4_PA_FIMC_LITE(1),
+				"exynos-fimc-lite.1", NULL),
 	{},
 };
 
-- 
1.7.9.5

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

* [PATCH RFC v2 11/15] ARM: dts: Add camera node exynos4.dtsi
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
                   ` (9 preceding siblings ...)
  2012-12-31 16:03 ` [PATCH RFC v2 10/15] ARM: EXYNOS4: Add OF_DEV_AUXDATA for FIMC, FIMC-LITE and CSIS Sylwester Nawrocki
@ 2012-12-31 16:03 ` Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 12/15] ARM: dts: Add ISP power domain node for Exynos4x12 Sylwester Nawrocki
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

This adds common FIMC device nodes for all Exynos4 SoCs.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/boot/dts/exynos4.dtsi |   64 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index b853b11..30364ce 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -28,6 +28,12 @@
 		spi0 = &spi_0;
 		spi1 = &spi_1;
 		spi2 = &spi_2;
+		csis0 = &csis_0;
+		csis1 = &csis_1;
+		fimc0 = &fimc_0;
+		fimc1 = &fimc_1;
+		fimc2 = &fimc_2;
+		fimc3 = &fimc_3;
 	};
 
 	pd_mfc: mfc-power-domain@10023C40 {
@@ -92,6 +98,64 @@
 		power-domain = <&pd_lcd0>;
 	};
 
+	camera {
+		compatible = "samsung,fimc", "simple-bus";
+		status = "disabled";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		fimc_0: fimc@11800000 {
+			compatible = "samsung,exynos4210-fimc";
+			reg = <0x11800000 0x1000>;
+			interrupts = <0 84 0>;
+			power-domain = <&pd_cam>;
+			status = "disabled";
+		};
+
+		fimc_1: fimc@11810000 {
+			compatible = "samsung,exynos4210-fimc";
+			reg = <0x11810000 0x1000>;
+			interrupts = <0 85 0>;
+			power-domain = <&pd_cam>;
+			status = "disabled";
+		};
+
+		fimc_2: fimc@11820000 {
+			compatible = "samsung,exynos4210-fimc";
+			reg = <0x11820000 0x1000>;
+			interrupts = <0 86 0>;
+			power-domain = <&pd_cam>;
+			status = "disabled";
+		};
+
+		fimc_3: fimc@11830000 {
+			compatible = "samsung,exynos4210-fimc";
+			reg = <0x11830000 0x1000>;
+			interrupts = <0 87 0>;
+			power-domain = <&pd_cam>;
+			status = "disabled";
+		};
+
+		csis_0: csis@11880000 {
+			compatible = "samsung,exynos4210-csis";
+			reg = <0x11880000 0x4000>;
+			interrupts = <0 78 0>;
+			max-data-lanes = <4>;
+			power-domain = <&pd_cam>;
+			status = "disabled";
+		};
+
+		csis_1: csis@11890000 {
+			compatible = "samsung,exynos4210-csis";
+			reg = <0x11890000 0x4000>;
+			interrupts = <0 80 0>;
+			max-data-lanes = <2>;
+			power-domain = <&pd_cam>;
+			status = "disabled";
+		};
+	};
+
 	watchdog@10060000 {
 		compatible = "samsung,s3c2410-wdt";
 		reg = <0x10060000 0x100>;
-- 
1.7.9.5

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

* [PATCH RFC v2 12/15] ARM: dts: Add ISP power domain node for Exynos4x12
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
                   ` (10 preceding siblings ...)
  2012-12-31 16:03 ` [PATCH RFC v2 11/15] ARM: dts: Add camera node exynos4.dtsi Sylwester Nawrocki
@ 2012-12-31 16:03 ` Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 13/15] ARM: dts: Add FIMC and MIPI CSIS device nodes " Sylwester Nawrocki
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

The ISP power domain is a common power domain for fimc-lite
and fimc-is (ISP) devices.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/boot/dts/exynos4x12.dtsi |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi
index 179a62e..9c809b72 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -28,6 +28,11 @@
 		pinctrl3 = &pinctrl_3;
 	};
 
+	pd_isp: isp-power-domain@10023CA0 {
+		compatible = "samsung,exynos4210-pd";
+		reg = <0x10023CA0 0x20>;
+	};
+
 	combiner:interrupt-controller@10440000 {
 		interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
 			     <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
-- 
1.7.9.5

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

* [PATCH RFC v2 13/15] ARM: dts: Add FIMC and MIPI CSIS device nodes for Exynos4x12
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
                   ` (11 preceding siblings ...)
  2012-12-31 16:03 ` [PATCH RFC v2 12/15] ARM: dts: Add ISP power domain node for Exynos4x12 Sylwester Nawrocki
@ 2012-12-31 16:03 ` Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 14/15] ARM: dts: Add camera pinctrl nodes for Exynos4x12 SoCs Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 15/15] ARM: dts: Add camera device nodes nodes for PQ board Sylwester Nawrocki
  14 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

Add common camera node and fimc nodes specific to Exynos4212 and
Exynos4412 SoCs. fimc-is is a node for the Exynos4x12 FIMC-IS
subsystem and fimc-lite nodes are created as its child nodes,
among others due to FIMC-LITE device dependencies on FIMC-IS
related clocks.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/boot/dts/exynos4x12.dtsi |   47 +++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi
index 9c809b72..06fde30 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -26,6 +26,8 @@
 		pinctrl1 = &pinctrl_1;
 		pinctrl2 = &pinctrl_2;
 		pinctrl3 = &pinctrl_3;
+		fimc-lite0 = &fimc_lite_0;
+		fimc-lite1 = &fimc_lite_1;
 	};
 
 	pd_isp: isp-power-domain@10023CA0 {
@@ -71,4 +73,49 @@
 		reg = <0x106E0000 0x1000>;
 		interrupts = <0 72 0>;
 	};
+
+	camera {
+		fimc_0: fimc@11800000 {
+			compatible = "samsung,exynos4212-fimc";
+		};
+
+		fimc_1: fimc@11810000 {
+			compatible = "samsung,exynos4212-fimc";
+		};
+
+		fimc_2: fimc@11820000 {
+			compatible = "samsung,exynos4212-fimc";
+		};
+
+		fimc_3: fimc@11830000 {
+			compatible = "samsung,exynos4212-fimc";
+		};
+
+		fimc_is: fimc-is@12000000 {
+			compatible = "samsung,exynos4212-fimc-is", "simple-bus";
+			reg = <0x12000000 0x260000>;
+			interrupts = <0 90 0>, <0 95 0>;
+			power-domain = <&pd_isp>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			fimc_lite_0: fimc_lite@12390000 {
+				compatible = "samsung,exynos4212-fimc-lite";
+				reg = <0x12390000 0x1000>;
+				interrupts = <0 125 0>;
+				power-domain = <&pd_isp>;
+				status = "disabled";
+			};
+
+			fimc_lite_1: fimc_lite@123A0000 {
+				compatible = "samsung,exynos4212-fimc-lite";
+				reg = <0x123A0000 0x1000>;
+				interrupts = <0 126 0>;
+				power-domain = <&pd_isp>;
+				status = "disabled";
+			};
+		};
+	};
 };
-- 
1.7.9.5

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

* [PATCH RFC v2 14/15] ARM: dts: Add camera pinctrl nodes for Exynos4x12 SoCs
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
                   ` (12 preceding siblings ...)
  2012-12-31 16:03 ` [PATCH RFC v2 13/15] ARM: dts: Add FIMC and MIPI CSIS device nodes " Sylwester Nawrocki
@ 2012-12-31 16:03 ` Sylwester Nawrocki
  2012-12-31 16:03 ` [PATCH RFC v2 15/15] ARM: dts: Add camera device nodes nodes for PQ board Sylwester Nawrocki
  14 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

Add separate nodes for the CAMCLK pin and turn off pull-up on camera
port A. Default driver strength for CAMCLK pin is increased to maximum.
The driver strength change can be moved to board specific part if it
is considered more appropriate.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/boot/dts/exynos4x12-pinctrl.dtsi |   33 +++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi b/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
index 56f4669..e3225d0 100644
--- a/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
+++ b/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
@@ -401,15 +401,28 @@
 			samsung,pin-drv = <0>;
 		};
 
-		cam_port_a: cam-port-a {
+		cam_port_a_io: cam-port-a-io {
 			samsung,pins = "gpj0-0", "gpj0-1", "gpj0-2", "gpj0-3",
 					"gpj0-4", "gpj0-5", "gpj0-6", "gpj0-7",
-					"gpj1-0", "gpj1-1", "gpj1-2", "gpj1-3",
-					"gpj1-4";
+					"gpj1-0", "gpj1-1", "gpj1-2", "gpj1-4";
 			samsung,pin-function = <2>;
-			samsung,pin-pud = <3>;
+			samsung,pin-pud = <0>;
 			samsung,pin-drv = <0>;
 		};
+
+		cam_port_a_clk_active: cam-port-a-clk-active {
+			samsung,pins = "gpj1-3";
+			samsung,pin-function = <2>;
+			samsung,pin-pud = <0>;
+			samsung,pin-drv = <3>;
+		};
+
+		cam_port_a_clk_idle: cam-port-a-clk-idle {
+			samsung,pins = "gpj1-3";
+			samsung,pin-function = <2>;
+			samsung,pin-pud = <0>;
+			samsung,pin-drv = <3>;
+		};
 	};
 
 	pinctrl@11000000 {
@@ -834,11 +847,17 @@
 			samsung,pin-drv = <0>;
 		};
 
-		cam_port_b: cam-port-b {
+		cam_port_b_io: cam-port-b-io {
 			samsung,pins = "gpm0-0", "gpm0-1", "gpm0-2", "gpm0-3",
 					"gpm0-4", "gpm0-5", "gpm0-6", "gpm0-7",
-					"gpm1-0", "gpm1-1", "gpm2-0", "gpm2-1",
-					"gpm2-2";
+					"gpm1-0", "gpm1-1", "gpm2-0", "gpm2-1";
+			samsung,pin-function = <3>;
+			samsung,pin-pud = <3>;
+			samsung,pin-drv = <0>;
+		};
+
+		cam_port_b_clk: cam-port-b-clk {
+			samsung,pins = "gpm2-2";
 			samsung,pin-function = <3>;
 			samsung,pin-pud = <3>;
 			samsung,pin-drv = <0>;
-- 
1.7.9.5

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

* [PATCH RFC v2 15/15] ARM: dts: Add camera device nodes nodes for PQ board
  2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
                   ` (13 preceding siblings ...)
  2012-12-31 16:03 ` [PATCH RFC v2 14/15] ARM: dts: Add camera pinctrl nodes for Exynos4x12 SoCs Sylwester Nawrocki
@ 2012-12-31 16:03 ` Sylwester Nawrocki
  14 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2012-12-31 16:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss,
	linux-samsung-soc, Sylwester Nawrocki

This patch adds all nodes for camera devices on example Exynos4412 SoC
based board. This is all what's required in the board dts file to enable
rear facing camera (S5C73M3 sensor).

The aliases node contains entries required for the camera processing
data path entity drivers.

The sensor nodes use standard port/remote-endpoint nodes convention.
Internal SoC links between entities are not specified this way and
are coded in the driver instead, as it seemed more reasonable.

The S5C73M3 sensor uses two control buses: I2C and SPI. There are
two, i2c_0 and spi_1 bus controller child nodes assigned to it.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/boot/dts/exynos4412-slp_pq.dts |  133 +++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4412-slp_pq.dts b/arch/arm/boot/dts/exynos4412-slp_pq.dts
index 3a5782d..97a4c2c 100644
--- a/arch/arm/boot/dts/exynos4412-slp_pq.dts
+++ b/arch/arm/boot/dts/exynos4412-slp_pq.dts
@@ -101,6 +101,34 @@
 		};
 	};
 
+	i2c_0: i2c@13860000 {
+		samsung,i2c-sda-delay = <100>;
+		samsung,i2c-slave-addr = <0x10>;
+		samsung,i2c-max-bus-freq = <400000>;
+		pinctrl-0 = <&i2c0_bus>;
+		status = "okay";
+
+		s5c73m3@3c {
+			compatible = "samsung,s5c73m3";
+			reg = <0x3c>;
+			gpios = <&gpm0 1 1>, /* ISP_STANDBY */
+				<&gpf1 3 1>; /* ISP_RESET */
+			vdd-int-supply = <&buck9_reg>;
+			vddio-cis-supply = <&ldo9_reg>;
+			vdda-supply = <&ldo17_reg>;
+			vddio-host-supply = <&ldo18_reg>;
+			vdd-af-supply = <&cam_af_reg>;
+			vdd-reg-supply = <&cam_io_reg>;
+			clock-frequency = <24000000>;
+
+			port {
+				s5c73m3_ep: endpoint {
+					remote-endpoint = <&csis0_ep>;
+				};
+			};
+		};
+	};
+
 	i2c@13890000 {
 		samsung,i2c-sda-delay = <100>;
 		samsung,i2c-slave-addr = <0x10>;
@@ -411,6 +439,34 @@
 		enable-active-high;
 	};
 
+	cam_af_reg: voltage-regulator@2 {
+	        compatible = "regulator-fixed";
+		regulator-name = "CAM_AF";
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+		gpio = <&gpm0 4 0>;
+		enable-active-high;
+	};
+
+	cam_io_reg: voltage-regulator@3 {
+	        compatible = "regulator-fixed";
+		regulator-name = "CAM_SENSOR_A";
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+		gpio = <&gpm0 2 0>;
+		enable-active-high;
+	};
+
+	cam_isp_core_reg: voltage-regulator@4 {
+	        compatible = "regulator-fixed";
+		regulator-name = "CAM_ISP_CORE_1.2V_EN";
+		regulator-min-microvolt = <1200000>;
+		regulator-max-microvolt = <1200000>;
+		gpio = <&gpm0 3 0>;
+		enable-active-high;
+		regulator-always-on;
+	};
+
 	fimd0_lcd: panel {
 		compatible = "s6e8ax0";
 		reset-gpio = <&gpy4 5 0>;
@@ -462,4 +518,81 @@
 		vusb_d-supply = <&ldo15_reg>;
 		vusb_a-supply = <&ldo12_reg>;
 	};
+
+	spi_1: spi@13930000 {
+		pinctrl-names = "default";
+		pinctrl-0 = <&spi1_bus>;
+		status = "okay";
+
+		s5c73m3_spi: s5c73m3 {
+			compatible = "samsung,s5c73m3";
+			spi-max-frequency = <50000000>;
+			reg = <0>;
+			controller-data {
+				cs-gpio = <&gpb 5 0>;
+				samsung,spi-feedback-delay = <2>;
+			};
+		};
+	};
+
+	camera {
+		compatible = "samsung,fimc", "simple-bus";
+		status = "okay";
+
+		pinctrl-names = "default", "inactive";
+		pinctrl-0 = <&cam_port_a_clk_active>;
+		pinctrl-1 = <&cam_port_a_clk_idle>;
+
+		fimc_0: fimc@11800000 {
+			status = "okay";
+		};
+
+		fimc_1: fimc@11810000 {
+			status = "okay";
+		};
+
+		fimc_2: fimc@11820000 {
+			status = "okay";
+		};
+
+		fimc_3: fimc@11830000 {
+			status = "okay";
+		};
+
+		csis_0: csis@11880000 {
+			status = "okay";
+			vddcore-supply = <&ldo8_reg>;
+			vddio-supply = <&ldo10_reg>;
+			clock-frequency = <160000000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			port {
+				reg = <3>;  /* Camera C(3) MIPI */
+				csis0_ep: endpoint {
+					remote-endpoint = <&s5c73m3_ep>;
+					data-lanes = <1>, <2>, <3>, <4>;
+					samsung,csis-hs-settle = <12>;
+				};
+			};
+		};
+
+		csis_1: csis@11890000 {
+			vddcore-supply = <&ldo8_reg>;
+			vddio-supply = <&ldo10_reg>;
+			samsung,csis-hs-settle = <18>;
+			data-lanes = <1>;
+		};
+
+		fimc-is@12000000 {
+			status = "okay";
+
+			fimc_lite_0: fimc_lite@12390000 {
+				status = "okay";
+			};
+
+			fimc_lite_1: fimc_lite@123A0000 {
+				status = "okay";
+			};
+		};
+	};
 };
-- 
1.7.9.5

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

* Re: [PATCH RFC v2 01/15] [media] Add common video interfaces OF bindings documentation
  2012-12-31 16:02 ` [PATCH RFC v2 01/15] [media] Add common video interfaces OF bindings documentation Sylwester Nawrocki
@ 2013-01-02 11:31   ` Guennadi Liakhovetski
  2013-01-02 21:51     ` Sylwester Nawrocki
  2013-01-03 17:03   ` [PATCH RFC v3 " Sylwester Nawrocki
  1 sibling, 1 reply; 31+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-02 11:31 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-media, grant.likely, rob.herring, thomas.abraham, t.figa,
	sw0312.kim, kyungmin.park, devicetree-discuss, linux-samsung-soc

Hi Sylwester

Thanks for picking up these patches! In general both look good to me, just 
a couple of nit-picks, that I couldn't help remarking:-)

On Mon, 31 Dec 2012, Sylwester Nawrocki wrote:

> From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> 
> This patch adds a document describing common OF bindings for video
> capture, output and video processing devices. It is curently mainly
> focused on video capture devices, with data busses defined by
> standards like ITU-R BT.656 or MIPI-CSI2.
> It also documents a method of describing data links between devices.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>
> 
> ---
> 
> This is basically a resend of my previous version of this patch [1],
> with just a few typo/grammar issue corrections.
> 
> [1] http://patchwork.linuxtv.org/patch/15911/
> ---
>  .../devicetree/bindings/media/video-interfaces.txt |  198 ++++++++++++++++++++
>  1 file changed, 198 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/video-interfaces.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
> new file mode 100644
> index 0000000..d1eea35
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -0,0 +1,198 @@
> +Common bindings for video data receiver and transmitter interfaces
> +
> +General concept
> +---------------
> +
> +Video data pipelines usually consist of external devices, e.g. camera sensors,
> +controlled over an I2C, SPI or UART bus, and SoC internal IP blocks, including
> +video DMA engines and video data processors.
> +
> +SoC internal blocks are described by DT nodes, placed similarly to other SoC
> +blocks.  External devices are represented as child nodes of their respective
> +bus controller nodes, e.g. I2C.
> +
> +Data interfaces on all video devices are described by their child 'port' nodes.
> +Configuration of a port depends on other devices participating in the data
> +transfer and is described by 'endpoint' subnodes.
> +
> +dev {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	port@0 {
> +		endpoint@0 { ... };
> +		endpoint@1 { ... };
> +	};
> +	port@1 { ... };
> +};
> +
> +If a port can be configured to work with more than one other device on the same
> +bus, an 'endpoint' child node must be provided for each of them.  If more than
> +one port is present in a device node or there is more than one endpoint at a
> +port, a common scheme, using '#address-cells', '#size-cells' and 'reg' properties
> +is used.
> +
> +Two 'endpoint' nodes are linked with each other through their 'remote-endpoint'
> +phandles.  An endpoint subnode of a device contains all properties needed for
> +configuration of this device for data exchange with the other device.  In most
> +cases properties at the peer 'endpoint' nodes will be identical, however
> +they might need to be different when there is any signal modifications on the
> +bus between two devices, e.g. there are logic signal inverters on the lines.
> +
> +Required properties
> +-------------------
> +
> +If there is more than one 'port' or more than one 'endpoint' node following
> +properties are required in relevant parent node:
> +
> +- #address-cells : number of cells required to define port number, should be 1.
> +- #size-cells    : should be zero.
> +
> +Optional endpoint properties
> +----------------------------
> +
> +- remote-endpoint : phandle to an 'endpoint' subnode of the other device node.

This spacing before ":" looks strange to me. I personally prefer the 
normal English rule - "x: y," i.e. no space before and a space after, but 
I wouldn't remark on your choice of a space on each side in this specific 
case, if it was consistent. Whereas sometimes having one space and 
sometimes having none looks weird to me. I would go for "no space before 
':'" throughout this document.

> +- slave-mode : a boolean property, run the link in slave mode. Default is master
> +  mode.
> +- bus-width : number of data lines, valid for parallel buses.

As we discussed before, both "busses" and "buses" spellings are commonly 
used at different locations around the world, but I think we should stick 
to only one of them in a single document. It looks weird to have "buses" 
in one line and "busses" in the following one.

> +- data-shift: on parallel data busses, if bus-width is used to specify the
> +  number of data lines, data-shift can be used to specify which data lines are
> +  used, e.g. "bus-width=<10>; data-shift=<2>;" means, that lines 9:2 are used.
> +- hsync-active : active state of HSYNC signal, 0/1 for LOW/HIGH respectively.
> +- vsync-active : active state of VSYNC signal, 0/1 for LOW/HIGH respectively.
> +  Note, that if HSYNC and VSYNC polarities are not specified, embedded
> +  synchronization may be required, where supported.
> +- data-active : similar to HSYNC and VSYNC, specifies data line polarity.
> +- field-even-active: field signal level during the even field data transmission.
> +- pclk-sample : rising (1) or falling (0) edge to sample the pixel clock signal.

Yes, it was in my original document too, but don't we mean "sample data on 
rising (1) or falling (0) edge of the pixel clock signal?"

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

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

* Re: [PATCH RFC v2 02/15] [media] Add a V4L2 OF parser
  2012-12-31 16:03 ` [PATCH RFC v2 02/15] [media] Add a V4L2 OF parser Sylwester Nawrocki
@ 2013-01-02 11:58   ` Guennadi Liakhovetski
  2013-01-02 22:11     ` Sylwester Nawrocki
       [not found]   ` <1356969793-27268-3-git-send-email-s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
  1 sibling, 1 reply; 31+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-02 11:58 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-media, grant.likely, rob.herring, thomas.abraham, t.figa,
	sw0312.kim, kyungmin.park, devicetree-discuss, linux-samsung-soc

Hi Sylwester

Just one question to this one:

On Mon, 31 Dec 2012, Sylwester Nawrocki wrote:

> diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
> new file mode 100644
> index 0000000..cdac04b
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -0,0 +1,249 @@
> +/*
> + * V4L2 OF binding parsing library
> + *
> + * Copyright (C) 2012 Renesas Electronics Corp.
> + * Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>

Is slab.h really needed? I didn't have it in my version. Maybe you meant 
to include string.h for memset()?

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

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

* Re: [PATCH RFC v2 01/15] [media] Add common video interfaces OF bindings documentation
  2013-01-02 11:31   ` Guennadi Liakhovetski
@ 2013-01-02 21:51     ` Sylwester Nawrocki
  2013-01-02 22:01       ` Guennadi Liakhovetski
  0 siblings, 1 reply; 31+ messages in thread
From: Sylwester Nawrocki @ 2013-01-02 21:51 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Sylwester Nawrocki, linux-media, grant.likely, rob.herring,
	thomas.abraham, t.figa, sw0312.kim, kyungmin.park,
	devicetree-discuss, linux-samsung-soc

Hi Guennadi,

On 01/02/2013 12:31 PM, Guennadi Liakhovetski wrote:
> Hi Sylwester
>
> Thanks for picking up these patches! In general both look good to me, just
> a couple of nit-picks, that I couldn't help remarking:-)

Sure, thanks again for the feedback.

> On Mon, 31 Dec 2012, Sylwester Nawrocki wrote:
>
>> From: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
>>
>> This patch adds a document describing common OF bindings for video
>> capture, output and video processing devices. It is curently mainly
>> focused on video capture devices, with data busses defined by
>> standards like ITU-R BT.656 or MIPI-CSI2.
>> It also documents a method of describing data links between devices.
>>
>> Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
>> Signed-off-by: Sylwester Nawrocki<s.nawrocki@samsung.com>
>> Reviewed-by: Stephen Warren<swarren@nvidia.com>
>>
>> ---
>>
>> This is basically a resend of my previous version of this patch [1],
>> with just a few typo/grammar issue corrections.
>>
>> [1] http://patchwork.linuxtv.org/patch/15911/
>> ---
>>   .../devicetree/bindings/media/video-interfaces.txt |  198 ++++++++++++++++++++
>>   1 file changed, 198 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/media/video-interfaces.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
>> new file mode 100644
>> index 0000000..d1eea35
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
>> @@ -0,0 +1,198 @@
>> +Common bindings for video data receiver and transmitter interfaces
>> +
>> +General concept
>> +---------------
>> +
>> +Video data pipelines usually consist of external devices, e.g. camera sensors,
>> +controlled over an I2C, SPI or UART bus, and SoC internal IP blocks, including
>> +video DMA engines and video data processors.
>> +
>> +SoC internal blocks are described by DT nodes, placed similarly to other SoC
>> +blocks.  External devices are represented as child nodes of their respective
>> +bus controller nodes, e.g. I2C.
>> +
>> +Data interfaces on all video devices are described by their child 'port' nodes.
>> +Configuration of a port depends on other devices participating in the data
>> +transfer and is described by 'endpoint' subnodes.
>> +
>> +dev {
>> +	#address-cells =<1>;
>> +	#size-cells =<0>;
>> +	port@0 {
>> +		endpoint@0 { ... };
>> +		endpoint@1 { ... };
>> +	};
>> +	port@1 { ... };
>> +};
>> +
>> +If a port can be configured to work with more than one other device on the same
>> +bus, an 'endpoint' child node must be provided for each of them.  If more than
>> +one port is present in a device node or there is more than one endpoint at a
>> +port, a common scheme, using '#address-cells', '#size-cells' and 'reg' properties
>> +is used.
>> +
>> +Two 'endpoint' nodes are linked with each other through their 'remote-endpoint'
>> +phandles.  An endpoint subnode of a device contains all properties needed for
>> +configuration of this device for data exchange with the other device.  In most
>> +cases properties at the peer 'endpoint' nodes will be identical, however
>> +they might need to be different when there is any signal modifications on the
>> +bus between two devices, e.g. there are logic signal inverters on the lines.
>> +
>> +Required properties
>> +-------------------
>> +
>> +If there is more than one 'port' or more than one 'endpoint' node following
>> +properties are required in relevant parent node:
>> +
>> +- #address-cells : number of cells required to define port number, should be 1.
>> +- #size-cells    : should be zero.
>> +
>> +Optional endpoint properties
>> +----------------------------
>> +
>> +- remote-endpoint : phandle to an 'endpoint' subnode of the other device node.
>
> This spacing before ":" looks strange to me. I personally prefer the
> normal English rule - "x: y," i.e. no space before and a space after, but
> I wouldn't remark on your choice of a space on each side in this specific
> case, if it was consistent. Whereas sometimes having one space and
> sometimes having none looks weird to me. I would go for "no space before
> ':'" throughout this document.

Gah, it was so close! ;) Sorry about it, it looked more readable to me 
that way.
And I've checked other bindings' documentation and there was many files 
having
space on both sides of a colon. Nevertheless, I will change it back to the
original form.

>> +- slave-mode : a boolean property, run the link in slave mode. Default is master
>> +  mode.
>> +- bus-width : number of data lines, valid for parallel buses.
>
> As we discussed before, both "busses" and "buses" spellings are commonly
> used at different locations around the world, but I think we should stick
> to only one of them in a single document. It looks weird to have "buses"
> in one line and "busses" in the following one.

True, I think that was the one occurrence I'd noticed and have forgotten to
correct then. I'll fix it, thanks for pointing out.

>> +- data-shift: on parallel data busses, if bus-width is used to specify the
>> +  number of data lines, data-shift can be used to specify which data lines are
>> +  used, e.g. "bus-width=<10>; data-shift=<2>;" means, that lines 9:2 are used.
>> +- hsync-active : active state of HSYNC signal, 0/1 for LOW/HIGH respectively.
>> +- vsync-active : active state of VSYNC signal, 0/1 for LOW/HIGH respectively.
>> +  Note, that if HSYNC and VSYNC polarities are not specified, embedded
>> +  synchronization may be required, where supported.
>> +- data-active : similar to HSYNC and VSYNC, specifies data line polarity.
>> +- field-even-active: field signal level during the even field data transmission.
>> +- pclk-sample : rising (1) or falling (0) edge to sample the pixel clock signal.
>
> Yes, it was in my original document too, but don't we mean "sample data on
> rising (1) or falling (0) edge of the pixel clock signal?"

Oops, I've managed to overlooked this. Certainly, it wasn't supposed to mean
sampling the clock signal. BTW, I had some doubts about this property. 
On the
transmitter side we more care about driving, rather than sampling data. And
usually when a transmitter drives data line at one clock edge type (e.g. 
rising)
then the receiver samples data on the other edge (e.g. falling).

In the display timing bindings there is a definitions like:

+ - pixelclk-active: with
+			- active high = drive pixel data on rising edge/
+					sample data on falling edge
+			- active low  = drive pixel data on falling edge/
+					sample data on rising edge
+			- ignored     = ignored

where:

+    <1>: high active
+    <0>: low active
+    omitted: not used on hardware


Then in our case, e.g. pclk-sample = <1>; on the transmitter side would mean
the receiver, which also has same pclk-sample = <1>; specified in its node,
has to sample data on rising clock edge and the transmitter is driving data
on falling edge, right ?

---

Thanks,
Sylwester

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

* Re: [PATCH RFC v2 01/15] [media] Add common video interfaces OF bindings documentation
  2013-01-02 21:51     ` Sylwester Nawrocki
@ 2013-01-02 22:01       ` Guennadi Liakhovetski
  0 siblings, 0 replies; 31+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-02 22:01 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: Sylwester Nawrocki, linux-media, grant.likely, rob.herring,
	thomas.abraham, t.figa, sw0312.kim, kyungmin.park,
	devicetree-discuss, linux-samsung-soc

Hi Sylwester

On Wed, 2 Jan 2013, Sylwester Nawrocki wrote:

> Hi Guennadi,
> 
> On 01/02/2013 12:31 PM, Guennadi Liakhovetski wrote:
> > Hi Sylwester
> > 
> > Thanks for picking up these patches! In general both look good to me, just
> > a couple of nit-picks, that I couldn't help remarking:-)
> 
> Sure, thanks again for the feedback.
> 
> > On Mon, 31 Dec 2012, Sylwester Nawrocki wrote:
> > 
> > > From: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
> > > 
> > > This patch adds a document describing common OF bindings for video
> > > capture, output and video processing devices. It is curently mainly
> > > focused on video capture devices, with data busses defined by
> > > standards like ITU-R BT.656 or MIPI-CSI2.
> > > It also documents a method of describing data links between devices.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
> > > Signed-off-by: Sylwester Nawrocki<s.nawrocki@samsung.com>
> > > Reviewed-by: Stephen Warren<swarren@nvidia.com>
> > > 
> > > ---
> > > 
> > > This is basically a resend of my previous version of this patch [1],
> > > with just a few typo/grammar issue corrections.
> > > 
> > > [1] http://patchwork.linuxtv.org/patch/15911/
> > > ---
> > >   .../devicetree/bindings/media/video-interfaces.txt |  198
> > > ++++++++++++++++++++
> > >   1 file changed, 198 insertions(+)
> > >   create mode 100644
> > > Documentation/devicetree/bindings/media/video-interfaces.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt
> > > b/Documentation/devicetree/bindings/media/video-interfaces.txt
> > > new file mode 100644
> > > index 0000000..d1eea35
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> > > @@ -0,0 +1,198 @@
> > > +Common bindings for video data receiver and transmitter interfaces
> > > +
> > > +General concept
> > > +---------------
> > > +
> > > +Video data pipelines usually consist of external devices, e.g. camera
> > > sensors,
> > > +controlled over an I2C, SPI or UART bus, and SoC internal IP blocks,
> > > including
> > > +video DMA engines and video data processors.
> > > +
> > > +SoC internal blocks are described by DT nodes, placed similarly to other
> > > SoC
> > > +blocks.  External devices are represented as child nodes of their
> > > respective
> > > +bus controller nodes, e.g. I2C.
> > > +
> > > +Data interfaces on all video devices are described by their child 'port'
> > > nodes.
> > > +Configuration of a port depends on other devices participating in the
> > > data
> > > +transfer and is described by 'endpoint' subnodes.
> > > +
> > > +dev {
> > > +	#address-cells =<1>;
> > > +	#size-cells =<0>;
> > > +	port@0 {
> > > +		endpoint@0 { ... };
> > > +		endpoint@1 { ... };
> > > +	};
> > > +	port@1 { ... };
> > > +};
> > > +
> > > +If a port can be configured to work with more than one other device on
> > > the same
> > > +bus, an 'endpoint' child node must be provided for each of them.  If more
> > > than
> > > +one port is present in a device node or there is more than one endpoint
> > > at a
> > > +port, a common scheme, using '#address-cells', '#size-cells' and 'reg'
> > > properties
> > > +is used.
> > > +
> > > +Two 'endpoint' nodes are linked with each other through their
> > > 'remote-endpoint'
> > > +phandles.  An endpoint subnode of a device contains all properties needed
> > > for
> > > +configuration of this device for data exchange with the other device.  In
> > > most
> > > +cases properties at the peer 'endpoint' nodes will be identical, however
> > > +they might need to be different when there is any signal modifications on
> > > the
> > > +bus between two devices, e.g. there are logic signal inverters on the
> > > lines.
> > > +
> > > +Required properties
> > > +-------------------
> > > +
> > > +If there is more than one 'port' or more than one 'endpoint' node
> > > following
> > > +properties are required in relevant parent node:
> > > +
> > > +- #address-cells : number of cells required to define port number, should
> > > be 1.
> > > +- #size-cells    : should be zero.
> > > +
> > > +Optional endpoint properties
> > > +----------------------------
> > > +
> > > +- remote-endpoint : phandle to an 'endpoint' subnode of the other device
> > > node.
> > 
> > This spacing before ":" looks strange to me. I personally prefer the
> > normal English rule - "x: y," i.e. no space before and a space after, but
> > I wouldn't remark on your choice of a space on each side in this specific
> > case, if it was consistent. Whereas sometimes having one space and
> > sometimes having none looks weird to me. I would go for "no space before
> > ':'" throughout this document.
> 
> Gah, it was so close! ;) Sorry about it, it looked more readable to me that
> way.
> And I've checked other bindings' documentation and there was many files having
> space on both sides of a colon. Nevertheless, I will change it back to the
> original form.
> 
> > > +- slave-mode : a boolean property, run the link in slave mode. Default is
> > > master
> > > +  mode.
> > > +- bus-width : number of data lines, valid for parallel buses.
> > 
> > As we discussed before, both "busses" and "buses" spellings are commonly
> > used at different locations around the world, but I think we should stick
> > to only one of them in a single document. It looks weird to have "buses"
> > in one line and "busses" in the following one.
> 
> True, I think that was the one occurrence I'd noticed and have forgotten to
> correct then. I'll fix it, thanks for pointing out.

I think there were at least 2 of them, but I might be wrong, a grep will 
tell with certainty :-)

> > > +- data-shift: on parallel data busses, if bus-width is used to specify
> > > the
> > > +  number of data lines, data-shift can be used to specify which data
> > > lines are
> > > +  used, e.g. "bus-width=<10>; data-shift=<2>;" means, that lines 9:2 are
> > > used.
> > > +- hsync-active : active state of HSYNC signal, 0/1 for LOW/HIGH
> > > respectively.
> > > +- vsync-active : active state of VSYNC signal, 0/1 for LOW/HIGH
> > > respectively.
> > > +  Note, that if HSYNC and VSYNC polarities are not specified, embedded
> > > +  synchronization may be required, where supported.
> > > +- data-active : similar to HSYNC and VSYNC, specifies data line polarity.
> > > +- field-even-active: field signal level during the even field data
> > > transmission.
> > > +- pclk-sample : rising (1) or falling (0) edge to sample the pixel clock
> > > signal.
> > 
> > Yes, it was in my original document too, but don't we mean "sample data on
> > rising (1) or falling (0) edge of the pixel clock signal?"
> 
> Oops, I've managed to overlooked this. Certainly, it wasn't supposed to mean
> sampling the clock signal. BTW, I had some doubts about this property. On the
> transmitter side we more care about driving, rather than sampling data. And
> usually when a transmitter drives data line at one clock edge type (e.g.
> rising)
> then the receiver samples data on the other edge (e.g. falling).
> 
> In the display timing bindings there is a definitions like:
> 
> + - pixelclk-active: with
> +			- active high = drive pixel data on rising edge/
> +					sample data on falling edge
> +			- active low  = drive pixel data on falling edge/
> +					sample data on rising edge
> +			- ignored     = ignored
> 
> where:
> 
> +    <1>: high active
> +    <0>: low active
> +    omitted: not used on hardware
> 
> 
> Then in our case, e.g. pclk-sample = <1>; on the transmitter side would mean
> the receiver, which also has same pclk-sample = <1>; specified in its node,
> has to sample data on rising clock edge and the transmitter is driving data
> on falling edge, right ?

That's also what seems logical to me. We can rephrase it to "should be 
sampled (by the receiver) on the rising edge."

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

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

* Re: [PATCH RFC v2 02/15] [media] Add a V4L2 OF parser
  2013-01-02 11:58   ` Guennadi Liakhovetski
@ 2013-01-02 22:11     ` Sylwester Nawrocki
  0 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2013-01-02 22:11 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Sylwester Nawrocki, linux-media, grant.likely, rob.herring,
	thomas.abraham, t.figa, sw0312.kim, kyungmin.park,
	devicetree-discuss, linux-samsung-soc

Hi Guennadi,

On 01/02/2013 12:58 PM, Guennadi Liakhovetski wrote:
>> --- /dev/null
>> +++ b/drivers/media/v4l2-core/v4l2-of.c
>> @@ -0,0 +1,249 @@
>> +/*
>> + * V4L2 OF binding parsing library
>> + *
>> + * Copyright (C) 2012 Renesas Electronics Corp.
>> + * Author: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of version 2 of the GNU General Public License as
>> + * published by the Free Software Foundation.
>> + */
>> +#include<linux/kernel.h>
>> +#include<linux/module.h>
>> +#include<linux/of.h>
>> +#include<linux/slab.h>
>
> Is slab.h really needed? I didn't have it in my version. Maybe you meant
> to include string.h for memset()?

I don't think it is needed, it looks like my mistake. I'll check it again
and replace it with string.h.

I've also noticed there are EXPORT_SYMBOL() missing for the first two 
functions
in this file. I'll fix it in next version.

---

Thanks,
Sylwester

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

* [PATCH RFC v3 01/15] [media] Add common video interfaces OF bindings documentation
  2012-12-31 16:02 ` [PATCH RFC v2 01/15] [media] Add common video interfaces OF bindings documentation Sylwester Nawrocki
  2013-01-02 11:31   ` Guennadi Liakhovetski
@ 2013-01-03 17:03   ` Sylwester Nawrocki
  2013-01-21 10:31     ` Hans Verkuil
  1 sibling, 1 reply; 31+ messages in thread
From: Sylwester Nawrocki @ 2013-01-03 17:03 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, laurent.pinchart, grant.likely, rob.herring,
	thomas.abraham, t.figa, sw0312.kim, kyungmin.park,
	devicetree-discuss, Sylwester Nawrocki

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

This patch adds a document describing common OF bindings for video
capture, output and video processing devices. It is curently mainly
focused on video capture devices, with data busses defined by
standards like ITU-R BT.656 or MIPI-CSI2.
It also documents a method of describing data links between devices.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Rob Herring <rob.herring@calxeda.com>
---

Changes since v2:

 - corrected pclk-sample property definition,
 - s/buses/busses,
 - whitespace changes.

 .../devicetree/bindings/media/video-interfaces.txt |  199 ++++++++++++++++++++
 1 file changed, 199 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/video-interfaces.txt

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
new file mode 100644
index 0000000..9e9e95a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -0,1 +1,199 @@
+Common bindings for video data receiver and transmitter interfaces
+
+General concept
+---------------
+
+Video data pipelines usually consist of external devices, e.g. camera sensors,
+controlled over an I2C, SPI or UART bus, and SoC internal IP blocks, including
+video DMA engines and video data processors.
+
+SoC internal blocks are described by DT nodes, placed similarly to other SoC
+blocks.  External devices are represented as child nodes of their respective
+bus controller nodes, e.g. I2C.
+
+Data interfaces on all video devices are described by their child 'port' nodes.
+Configuration of a port depends on other devices participating in the data
+transfer and is described by 'endpoint' subnodes.
+
+dev {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	port@0 {
+		endpoint@0 { ... };
+		endpoint@1 { ... };
+	};
+	port@1 { ... };
+};
+
+If a port can be configured to work with more than one other device on the same
+bus, an 'endpoint' child node must be provided for each of them.  If more than
+one port is present in a device node or there is more than one endpoint at a
+port, a common scheme, using '#address-cells', '#size-cells' and 'reg' properties
+is used.
+
+Two 'endpoint' nodes are linked with each other through their 'remote-endpoint'
+phandles.  An endpoint subnode of a device contains all properties needed for
+configuration of this device for data exchange with the other device.  In most
+cases properties at the peer 'endpoint' nodes will be identical, however
+they might need to be different when there is any signal modifications on the
+bus between two devices, e.g. there are logic signal inverters on the lines.
+
+Required properties
+-------------------
+
+If there is more than one 'port' or more than one 'endpoint' node following
+properties are required in relevant parent node:
+
+- #address-cells : number of cells required to define port number, should be 1.
+- #size-cells    : should be zero.
+
+Optional endpoint properties
+----------------------------
+
+- remote-endpoint: phandle to an 'endpoint' subnode of the other device node.
+- slave-mode: a boolean property, run the link in slave mode. Default is master
+  mode.
+- bus-width: number of data lines, valid for parallel busses.
+- data-shift: on parallel data busses, if bus-width is used to specify the
+  number of data lines, data-shift can be used to specify which data lines are
+  used, e.g. "bus-width=<10>; data-shift=<2>;" means, that lines 9:2 are used.
+- hsync-active: active state of HSYNC signal, 0/1 for LOW/HIGH respectively.
+- vsync-active: active state of VSYNC signal, 0/1 for LOW/HIGH respectively.
+  Note, that if HSYNC and VSYNC polarities are not specified, embedded
+  synchronization may be required, where supported.
+- data-active: similar to HSYNC and VSYNC, specifies data line polarity.
+- field-even-active: field signal level during the even field data transmission.
+- pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock
+  signal.
+- data-lanes: an array of physical data lane indexes. Position of an entry
+  determines logical lane number, while the value of an entry indicates physical
+  lane, e.g. for 2-lane MIPI CSI-2 bus we could have "data-lanes = <1>, <2>;",
+  assuming the clock lane is on hardware lane 0. This property is valid for
+  serial busses only (e.g. MIPI CSI-2).
+- clock-lanes: a number of physical lane used as a clock lane.
+- clock-noncontinuous: a boolean property to allow MIPI CSI-2 non-continuous
+  clock mode.
+
+Example
+-------
+
+The below example snippet describes two data pipelines.  ov772x and imx074 are
+camera sensors with parallel and serial (MIPI CSI-2) video bus respectively.
+Both sensors are on I2C control bus corresponding to i2c0 controller node.
+ov772x sensor is linked directly to the ceu0 video host interface.  imx074 is
+linked to ceu0 through MIPI CSI-2 receiver (csi2). ceu0 has a (single) DMA
+engine writing captured data to memory.  ceu0 node has single 'port' node which
+indicates at any time only one of following data pipeline can be active:
+ov772x -> ceu0 or imx074 -> csi2 -> ceu0.
+
+	ceu0: ceu@0xfe910000 {
+		compatible = "renesas,sh-mobile-ceu";
+		reg = <0xfe910000 0xa0>;
+		interrupts = <0x880>;
+
+		mclk: master_clock {
+			compatible = "renesas,ceu-clock";
+			#clock-cells = <1>;
+			clock-frequency = <50000000>;	/* Max clock frequency */
+			clock-output-names = "mclk";
+		};
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			ceu0_1: endpoint@1 {
+				reg = <1>;		/* Local endpoint # */
+				remote = <&ov772x_1_1>;	/* Remote phandle */
+				bus-width = <8>;	/* Used data lines */
+				data-shift = <0>;	/* Lines 7:0 are used */
+
+				/* If hsync-active/vsync-active are missing,
+				   embedded bt.605 sync is used */
+				hsync-active = <1>;	/* Active high */
+				vsync-active = <1>;	/* Active high */
+				data-active = <1>;	/* Active high */
+				pclk-sample = <1>;	/* Rising */
+			};
+
+			ceu0_0: endpoint@0 {
+				reg = <0>;
+				remote = <&csi2_2>;
+				immutable;
+			};
+		};
+	};
+
+	i2c0: i2c@0xfff20000 {
+		...
+		ov772x_1: camera@0x21 {
+			compatible = "omnivision,ov772x";
+			reg = <0x21>;
+			vddio-supply = <&regulator1>;
+			vddcore-supply = <&regulator2>;
+
+			clock-frequency = <20000000>;
+			clocks = <&mclk 0>;
+			clock-names = "xclk";
+
+			port {
+				/* With 1 endpoint per port no need in addresses. */
+				ov772x_1_1: endpoint {
+					bus-width = <8>;
+					remote-endpoint = <&ceu0_1>;
+					hsync-active = <1>;
+					vsync-active = <0>; /* Who came up with an
+							       inverter here ?... */
+					data-active = <1>;
+					pclk-sample = <1>;
+				};
+			};
+		};
+
+		imx074: camera@0x1a {
+			compatible = "sony,imx074";
+			reg = <0x1a>;
+			vddio-supply = <&regulator1>;
+			vddcore-supply = <&regulator2>;
+
+			clock-frequency = <30000000>;	/* Shared clock with ov772x_1 */
+			clocks = <&mclk 0>;
+			clock-names = "sysclk";		/* Assuming this is the
+							   name in the datasheet */
+			port {
+				imx074_1: endpoint {
+					clock-lanes = <0>;
+					data-lanes = <1>, <2>;
+					remote-endpoint = <&csi2_1>;
+				};
+			};
+		};
+	};
+
+	csi2: csi2@0xffc90000 {
+		compatible = "renesas,sh-mobile-csi2";
+		reg = <0xffc90000 0x1000>;
+		interrupts = <0x17a0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@1 {
+			compatible = "renesas,csi2c";	/* One of CSI2I and CSI2C. */
+			reg = <1>;			/* CSI-2 PHY #1 of 2: PHY_S,
+							   PHY_M has port address 0,
+							   is unused. */
+			csi2_1: endpoint {
+				clock-lanes = <0>;
+				data-lanes = <2>, <1>;
+				remote-endpoint = <&imx074_1>;
+			};
+		};
+		port@2 {
+			reg = <2>;			/* port 2: link to the CEU */
+
+			csi2_2: endpoint {
+				immutable;
+				remote-endpoint = <&ceu0_0>;
+			};
+		};
+	};
--
1.7.9.5

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

* [PATCH RFC v3 02/15] [media] Add a V4L2 OF parser
  2012-12-31 16:03 ` [PATCH RFC v2 02/15] [media] Add a V4L2 OF parser Sylwester Nawrocki
@ 2013-01-03 17:09       ` Sylwester Nawrocki
       [not found]   ` <1356969793-27268-3-git-send-email-s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
  1 sibling, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2013-01-03 17:09 UTC (permalink / raw)
  To: linux-media-u79uwXL29TY76Z2rM5mHXA
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	sw0312.kim-Sze3O3UU22JBDgjK7y7TUQ,
	rob.herring-bsGFqQB8/DxBDgjK7y7TUQ,
	kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, Sylwester Nawrocki,
	g.liakhovetski-Mmb7MZpHnFY

From: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>

Add a V4L2 OF parser, implementing bindings documented in
Documentation/devicetree/bindings/media/video-interfaces.txt.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
Signed-off-by: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
Changes since v2:
 - added missing EXPORT_SYMBOL for v4l2_of_parse_mipi_csi2()
   and v4l2_of_parse_parallel_bus() functions,
 - include string.h header instead of slab.h.

 drivers/media/v4l2-core/Makefile  |    3 +
 drivers/media/v4l2-core/v4l2-of.c |  251 +++++++++++++++++++++++++++++++++++++
 include/media/v4l2-of.h           |   79 ++++++++++++
 3 files changed, 333 insertions(+)
 create mode 100644 drivers/media/v4l2-core/v4l2-of.c
 create mode 100644 include/media/v4l2-of.h

diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index c2d61d4..00f64d6 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -9,6 +9,9 @@ videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
 ifeq ($(CONFIG_COMPAT),y)
   videodev-objs += v4l2-compat-ioctl32.o
 endif
+ifeq ($(CONFIG_OF),y)
+  videodev-objs += v4l2-of.o
+endif

 obj-$(CONFIG_VIDEO_DEV) += videodev.o v4l2-int-device.o
 obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
new file mode 100644
index 0000000..483245c
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -0,0 +1,251 @@
+/*
+ * V4L2 OF binding parsing library
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#include <media/v4l2-of.h>
+
+/**
+ * v4l2_of_parse_mipi_csi2() - parse MIPI CSI-2 bus properties
+ * @node: pointer to endpoint device_node
+ * @endpoint: pointer to v4l2_of_endpoint data structure
+ *
+ * Return: 0 on success or negative error value otherwise.
+ */
+int v4l2_of_parse_mipi_csi2(const struct device_node *node,
+			    struct v4l2_of_endpoint *endpoint)
+{
+	struct v4l2_of_mipi_csi2 *mipi_csi2 = &endpoint->mipi_csi_2;
+	u32 data_lanes[ARRAY_SIZE(mipi_csi2->data_lanes)];
+	struct property *prop;
+	const __be32 *lane = NULL;
+	u32 v;
+	int i = 0;
+
+	prop = of_find_property(node, "data-lanes", NULL);
+	if (!prop)
+		return -EINVAL;
+	do {
+		lane = of_prop_next_u32(prop, lane, &data_lanes[i]);
+	} while (lane && i++ < ARRAY_SIZE(data_lanes));
+
+	mipi_csi2->num_data_lanes = i;
+	while (i--)
+		mipi_csi2->data_lanes[i] = data_lanes[i];
+
+	if (!of_property_read_u32(node, "clock-lanes", &v))
+		mipi_csi2->clock_lane = v;
+
+	if (of_get_property(node, "clock-noncontinuous", &v))
+		endpoint->mbus_flags |= V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK;
+
+	return 0;
+}
+EXPORT_SYMBOL(v4l2_of_parse_mipi_csi2);
+
+/**
+ * v4l2_of_parse_parallel_bus() - parse parallel bus properties
+ * @node: pointer to endpoint device_node
+ * @endpoint: pointer to v4l2_of_endpoint data structure
+ */
+void v4l2_of_parse_parallel_bus(const struct device_node *node,
+				struct v4l2_of_endpoint *endpoint)
+{
+	unsigned int flags = 0;
+	u32 v;
+
+	if (WARN_ON(!endpoint))
+		return;
+
+	if (!of_property_read_u32(node, "hsync-active", &v))
+		flags |= v ? V4L2_MBUS_HSYNC_ACTIVE_HIGH :
+			V4L2_MBUS_HSYNC_ACTIVE_LOW;
+
+	if (!of_property_read_u32(node, "vsync-active", &v))
+		flags |= v ? V4L2_MBUS_VSYNC_ACTIVE_HIGH :
+			V4L2_MBUS_VSYNC_ACTIVE_LOW;
+
+	if (!of_property_read_u32(node, "pclk-sample", &v))
+		flags |= v ? V4L2_MBUS_PCLK_SAMPLE_RISING :
+			V4L2_MBUS_PCLK_SAMPLE_FALLING;
+
+	if (!of_property_read_u32(node, "field-even-active", &v))
+		flags |= v ? V4L2_MBUS_FIELD_EVEN_HIGH :
+			V4L2_MBUS_FIELD_EVEN_LOW;
+	if (flags)
+		endpoint->mbus_type = V4L2_MBUS_PARALLEL;
+	else
+		endpoint->mbus_type = V4L2_MBUS_BT656;
+
+	if (!of_property_read_u32(node, "data-active", &v))
+		flags |= v ? V4L2_MBUS_DATA_ACTIVE_HIGH :
+			V4L2_MBUS_DATA_ACTIVE_LOW;
+
+	if (of_get_property(node, "slave-mode", &v))
+		flags |= V4L2_MBUS_SLAVE;
+
+	if (!of_property_read_u32(node, "bus-width", &v))
+		endpoint->parallel.bus_width = v;
+
+	if (!of_property_read_u32(node, "data-shift", &v))
+		endpoint->parallel.data_shift = v;
+
+	endpoint->mbus_flags = flags;
+}
+EXPORT_SYMBOL(v4l2_of_parse_parallel_bus);
+
+/**
+ * v4l2_of_parse_endpoint() - parse all endpoint node properties
+ * @node: pointer to endpoint device_node
+ * @endpoint: pointer to v4l2_of_endpoint data structure
+ *
+ * All properties are optional. If none are found, we don't set any flags.
+ * This means the port has a static configuration and no properties have
+ * to be specified explicitly.
+ * If any properties that identify the bus as parallel are found and
+ * slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if we recognise
+ * the bus as serial CSI-2 and clock-noncontinuous isn't set, we set the
+ * V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag.
+ * The caller should hold a reference to @node.
+ */
+void v4l2_of_parse_endpoint(const struct device_node *node,
+			    struct v4l2_of_endpoint *endpoint)
+{
+	const struct device_node *port_node = of_get_parent(node);
+	bool data_lanes_present = false;
+
+	memset(endpoint, 0, sizeof(*endpoint));
+
+	endpoint->local_node = node;
+
+	/* Doesn't matter, whether the below two calls succeed */
+	of_property_read_u32(port_node, "reg", &endpoint->port);
+	of_property_read_u32(node, "reg", &endpoint->addr);
+
+	v4l2_of_parse_parallel_bus(node, endpoint);
+
+	/* If any parallel bus properties have been found, skip serial ones */
+	if (endpoint->parallel.bus_width || endpoint->parallel.data_shift ||
+	    endpoint->mbus_flags) {
+		/* Default parallel bus-master */
+		if (!(endpoint->mbus_flags & V4L2_MBUS_SLAVE))
+			endpoint->mbus_flags |= V4L2_MBUS_MASTER;
+		return;
+	}
+
+	endpoint->mbus_type = V4L2_MBUS_CSI2;
+
+	if (!v4l2_of_parse_mipi_csi2(node, endpoint))
+		data_lanes_present = true;
+
+	if ((endpoint->mipi_csi_2.clock_lane || data_lanes_present) &&
+	    !(endpoint->mbus_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK)) {
+		/* Default CSI-2: continuous clock */
+		endpoint->mbus_flags |= V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+	}
+}
+EXPORT_SYMBOL(v4l2_of_parse_endpoint);
+
+/*
+ * Return a refcounted next "endpoint" DT node. Contrary to the common OF
+ * practice, we do not drop the reference to previous, users have to do it
+ * themselves, when they're done with the node.
+ */
+struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
+					struct device_node *previous)
+{
+	struct device_node *child, *port;
+
+	if (!parent)
+		return NULL;
+
+	if (!previous) {
+		/*
+		 * If this is the first call, we have to find a port within this
+		 * node
+		 */
+		for_each_child_of_node(parent, port) {
+			if (!of_node_cmp(port->name, "port"))
+				break;
+		}
+		if (port) {
+			/* Found a port, get a link */
+			child = of_get_next_child(port, NULL);
+			of_node_put(port);
+		} else {
+			child = NULL;
+		}
+		if (!child)
+			pr_err("%s(): Invalid DT: %s has no link children!\n",
+			       __func__, parent->name);
+	} else {
+		port = of_get_parent(previous);
+		if (!port)
+			/* Hm, has someone given us the root node?... */
+			return NULL;
+
+		/* Avoid dropping previous refcount to 0 */
+		of_node_get(previous);
+		child = of_get_next_child(port, previous);
+		if (child) {
+			of_node_put(port);
+			return child;
+		}
+
+		/* No more links under this port, try the next one */
+		do {
+			port = of_get_next_child(parent, port);
+			if (!port)
+				return NULL;
+		} while (of_node_cmp(port->name, "port"));
+
+		/* Pick up the first link on this port */
+		child = of_get_next_child(port, NULL);
+		of_node_put(port);
+	}
+
+	return child;
+}
+EXPORT_SYMBOL(v4l2_of_get_next_endpoint);
+
+/**
+ * v4l2_of_get_remote_port_parent() - get remote port's parent node
+ * @node: pointer to local endpoint device_node
+ *
+ * Return: Remote device node associated with remote endpoint node linked
+ *	   to @node. Use of_node_put() on it when done.
+ */
+struct device_node *v4l2_of_get_remote_port_parent(
+			       const struct device_node *node)
+{
+	struct device_node *re, *tmp;
+
+	/* Get remote endpoint DT node. */
+	re = of_parse_phandle(node, "remote-endpoint", 0);
+	if (!re)
+		return NULL;
+
+	/* Remote port. */
+	tmp = of_get_parent(re);
+	of_node_put(re);
+	if (!tmp)
+		return NULL;
+
+	/* Remote device node. */
+	re = of_get_parent(tmp);
+	of_node_put(tmp);
+
+	return re;
+}
+EXPORT_SYMBOL(v4l2_of_get_remote_port_parent);
diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
new file mode 100644
index 0000000..1aba3b3
--- /dev/null
+++ b/include/media/v4l2-of.h
@@ -0,0 +1,79 @@
+/*
+ * V4L2 OF binding parsing library
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#ifndef _V4L2_OF_H
+#define _V4L2_OF_H
+
+#include <linux/list.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+
+#include <media/v4l2-mediabus.h>
+
+struct device_node;
+
+struct v4l2_of_mipi_csi2 {
+	unsigned char data_lanes[4];
+	unsigned char clock_lane;
+	unsigned short num_data_lanes;
+};
+
+struct v4l2_of_endpoint {
+	unsigned int port;
+	unsigned int addr;
+	struct list_head head;
+	const struct device_node *local_node;
+	const __be32 *remote;
+	enum v4l2_mbus_type mbus_type;
+	unsigned int mbus_flags;
+	union {
+		struct {
+			unsigned char bus_width;
+			unsigned char data_shift;
+		} parallel;
+		struct v4l2_of_mipi_csi2 mipi_csi_2;
+	};
+};
+
+#ifdef CONFIG_OF
+int v4l2_of_parse_mipi_csi2(const struct device_node *node,
+			    struct v4l2_of_endpoint *endpoint);
+void v4l2_of_parse_parallel_bus(const struct device_node *node,
+				struct v4l2_of_endpoint *endpoint);
+void v4l2_of_parse_endpoint(const struct device_node *node,
+			    struct v4l2_of_endpoint *link);
+struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
+					struct device_node *previous);
+struct device_node *v4l2_of_get_remote_port_parent(
+					const struct device_node *node);
+#else /* CONFIG_OF */
+
+static inline int v4l2_of_parse_endpoint(const struct device_node *node,
+					struct v4l2_of_endpoint *link)
+{
+	return -ENOSYS;
+}
+
+static inline struct device_node *v4l2_of_get_next_endpoint(
+					const struct device_node *parent,
+					struct device_node *previous)
+{
+	return NULL;
+}
+
+static inline struct device_node *v4l2_of_get_remote_endpoint(
+					const struct device_node *node)
+{
+	return NULL;
+}
+
+#endif /* CONFIG_OF */
+
+#endif /* _V4L2_OF_H */
--
1.7.9.5

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

* [PATCH RFC v3 02/15] [media] Add a V4L2 OF parser
@ 2013-01-03 17:09       ` Sylwester Nawrocki
  0 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2013-01-03 17:09 UTC (permalink / raw)
  To: linux-media
  Cc: g.liakhovetski, laurent.pinchart, grant.likely, rob.herring,
	thomas.abraham, t.figa, sw0312.kim, kyungmin.park,
	devicetree-discuss, Sylwester Nawrocki

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Add a V4L2 OF parser, implementing bindings documented in
Documentation/devicetree/bindings/media/video-interfaces.txt.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
Changes since v2:
 - added missing EXPORT_SYMBOL for v4l2_of_parse_mipi_csi2()
   and v4l2_of_parse_parallel_bus() functions,
 - include string.h header instead of slab.h.

 drivers/media/v4l2-core/Makefile  |    3 +
 drivers/media/v4l2-core/v4l2-of.c |  251 +++++++++++++++++++++++++++++++++++++
 include/media/v4l2-of.h           |   79 ++++++++++++
 3 files changed, 333 insertions(+)
 create mode 100644 drivers/media/v4l2-core/v4l2-of.c
 create mode 100644 include/media/v4l2-of.h

diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index c2d61d4..00f64d6 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -9,6 +9,9 @@ videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
 ifeq ($(CONFIG_COMPAT),y)
   videodev-objs += v4l2-compat-ioctl32.o
 endif
+ifeq ($(CONFIG_OF),y)
+  videodev-objs += v4l2-of.o
+endif

 obj-$(CONFIG_VIDEO_DEV) += videodev.o v4l2-int-device.o
 obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
new file mode 100644
index 0000000..483245c
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -0,0 +1,251 @@
+/*
+ * V4L2 OF binding parsing library
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#include <media/v4l2-of.h>
+
+/**
+ * v4l2_of_parse_mipi_csi2() - parse MIPI CSI-2 bus properties
+ * @node: pointer to endpoint device_node
+ * @endpoint: pointer to v4l2_of_endpoint data structure
+ *
+ * Return: 0 on success or negative error value otherwise.
+ */
+int v4l2_of_parse_mipi_csi2(const struct device_node *node,
+			    struct v4l2_of_endpoint *endpoint)
+{
+	struct v4l2_of_mipi_csi2 *mipi_csi2 = &endpoint->mipi_csi_2;
+	u32 data_lanes[ARRAY_SIZE(mipi_csi2->data_lanes)];
+	struct property *prop;
+	const __be32 *lane = NULL;
+	u32 v;
+	int i = 0;
+
+	prop = of_find_property(node, "data-lanes", NULL);
+	if (!prop)
+		return -EINVAL;
+	do {
+		lane = of_prop_next_u32(prop, lane, &data_lanes[i]);
+	} while (lane && i++ < ARRAY_SIZE(data_lanes));
+
+	mipi_csi2->num_data_lanes = i;
+	while (i--)
+		mipi_csi2->data_lanes[i] = data_lanes[i];
+
+	if (!of_property_read_u32(node, "clock-lanes", &v))
+		mipi_csi2->clock_lane = v;
+
+	if (of_get_property(node, "clock-noncontinuous", &v))
+		endpoint->mbus_flags |= V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK;
+
+	return 0;
+}
+EXPORT_SYMBOL(v4l2_of_parse_mipi_csi2);
+
+/**
+ * v4l2_of_parse_parallel_bus() - parse parallel bus properties
+ * @node: pointer to endpoint device_node
+ * @endpoint: pointer to v4l2_of_endpoint data structure
+ */
+void v4l2_of_parse_parallel_bus(const struct device_node *node,
+				struct v4l2_of_endpoint *endpoint)
+{
+	unsigned int flags = 0;
+	u32 v;
+
+	if (WARN_ON(!endpoint))
+		return;
+
+	if (!of_property_read_u32(node, "hsync-active", &v))
+		flags |= v ? V4L2_MBUS_HSYNC_ACTIVE_HIGH :
+			V4L2_MBUS_HSYNC_ACTIVE_LOW;
+
+	if (!of_property_read_u32(node, "vsync-active", &v))
+		flags |= v ? V4L2_MBUS_VSYNC_ACTIVE_HIGH :
+			V4L2_MBUS_VSYNC_ACTIVE_LOW;
+
+	if (!of_property_read_u32(node, "pclk-sample", &v))
+		flags |= v ? V4L2_MBUS_PCLK_SAMPLE_RISING :
+			V4L2_MBUS_PCLK_SAMPLE_FALLING;
+
+	if (!of_property_read_u32(node, "field-even-active", &v))
+		flags |= v ? V4L2_MBUS_FIELD_EVEN_HIGH :
+			V4L2_MBUS_FIELD_EVEN_LOW;
+	if (flags)
+		endpoint->mbus_type = V4L2_MBUS_PARALLEL;
+	else
+		endpoint->mbus_type = V4L2_MBUS_BT656;
+
+	if (!of_property_read_u32(node, "data-active", &v))
+		flags |= v ? V4L2_MBUS_DATA_ACTIVE_HIGH :
+			V4L2_MBUS_DATA_ACTIVE_LOW;
+
+	if (of_get_property(node, "slave-mode", &v))
+		flags |= V4L2_MBUS_SLAVE;
+
+	if (!of_property_read_u32(node, "bus-width", &v))
+		endpoint->parallel.bus_width = v;
+
+	if (!of_property_read_u32(node, "data-shift", &v))
+		endpoint->parallel.data_shift = v;
+
+	endpoint->mbus_flags = flags;
+}
+EXPORT_SYMBOL(v4l2_of_parse_parallel_bus);
+
+/**
+ * v4l2_of_parse_endpoint() - parse all endpoint node properties
+ * @node: pointer to endpoint device_node
+ * @endpoint: pointer to v4l2_of_endpoint data structure
+ *
+ * All properties are optional. If none are found, we don't set any flags.
+ * This means the port has a static configuration and no properties have
+ * to be specified explicitly.
+ * If any properties that identify the bus as parallel are found and
+ * slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if we recognise
+ * the bus as serial CSI-2 and clock-noncontinuous isn't set, we set the
+ * V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag.
+ * The caller should hold a reference to @node.
+ */
+void v4l2_of_parse_endpoint(const struct device_node *node,
+			    struct v4l2_of_endpoint *endpoint)
+{
+	const struct device_node *port_node = of_get_parent(node);
+	bool data_lanes_present = false;
+
+	memset(endpoint, 0, sizeof(*endpoint));
+
+	endpoint->local_node = node;
+
+	/* Doesn't matter, whether the below two calls succeed */
+	of_property_read_u32(port_node, "reg", &endpoint->port);
+	of_property_read_u32(node, "reg", &endpoint->addr);
+
+	v4l2_of_parse_parallel_bus(node, endpoint);
+
+	/* If any parallel bus properties have been found, skip serial ones */
+	if (endpoint->parallel.bus_width || endpoint->parallel.data_shift ||
+	    endpoint->mbus_flags) {
+		/* Default parallel bus-master */
+		if (!(endpoint->mbus_flags & V4L2_MBUS_SLAVE))
+			endpoint->mbus_flags |= V4L2_MBUS_MASTER;
+		return;
+	}
+
+	endpoint->mbus_type = V4L2_MBUS_CSI2;
+
+	if (!v4l2_of_parse_mipi_csi2(node, endpoint))
+		data_lanes_present = true;
+
+	if ((endpoint->mipi_csi_2.clock_lane || data_lanes_present) &&
+	    !(endpoint->mbus_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK)) {
+		/* Default CSI-2: continuous clock */
+		endpoint->mbus_flags |= V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+	}
+}
+EXPORT_SYMBOL(v4l2_of_parse_endpoint);
+
+/*
+ * Return a refcounted next "endpoint" DT node. Contrary to the common OF
+ * practice, we do not drop the reference to previous, users have to do it
+ * themselves, when they're done with the node.
+ */
+struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
+					struct device_node *previous)
+{
+	struct device_node *child, *port;
+
+	if (!parent)
+		return NULL;
+
+	if (!previous) {
+		/*
+		 * If this is the first call, we have to find a port within this
+		 * node
+		 */
+		for_each_child_of_node(parent, port) {
+			if (!of_node_cmp(port->name, "port"))
+				break;
+		}
+		if (port) {
+			/* Found a port, get a link */
+			child = of_get_next_child(port, NULL);
+			of_node_put(port);
+		} else {
+			child = NULL;
+		}
+		if (!child)
+			pr_err("%s(): Invalid DT: %s has no link children!\n",
+			       __func__, parent->name);
+	} else {
+		port = of_get_parent(previous);
+		if (!port)
+			/* Hm, has someone given us the root node?... */
+			return NULL;
+
+		/* Avoid dropping previous refcount to 0 */
+		of_node_get(previous);
+		child = of_get_next_child(port, previous);
+		if (child) {
+			of_node_put(port);
+			return child;
+		}
+
+		/* No more links under this port, try the next one */
+		do {
+			port = of_get_next_child(parent, port);
+			if (!port)
+				return NULL;
+		} while (of_node_cmp(port->name, "port"));
+
+		/* Pick up the first link on this port */
+		child = of_get_next_child(port, NULL);
+		of_node_put(port);
+	}
+
+	return child;
+}
+EXPORT_SYMBOL(v4l2_of_get_next_endpoint);
+
+/**
+ * v4l2_of_get_remote_port_parent() - get remote port's parent node
+ * @node: pointer to local endpoint device_node
+ *
+ * Return: Remote device node associated with remote endpoint node linked
+ *	   to @node. Use of_node_put() on it when done.
+ */
+struct device_node *v4l2_of_get_remote_port_parent(
+			       const struct device_node *node)
+{
+	struct device_node *re, *tmp;
+
+	/* Get remote endpoint DT node. */
+	re = of_parse_phandle(node, "remote-endpoint", 0);
+	if (!re)
+		return NULL;
+
+	/* Remote port. */
+	tmp = of_get_parent(re);
+	of_node_put(re);
+	if (!tmp)
+		return NULL;
+
+	/* Remote device node. */
+	re = of_get_parent(tmp);
+	of_node_put(tmp);
+
+	return re;
+}
+EXPORT_SYMBOL(v4l2_of_get_remote_port_parent);
diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
new file mode 100644
index 0000000..1aba3b3
--- /dev/null
+++ b/include/media/v4l2-of.h
@@ -0,0 +1,79 @@
+/*
+ * V4L2 OF binding parsing library
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#ifndef _V4L2_OF_H
+#define _V4L2_OF_H
+
+#include <linux/list.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+
+#include <media/v4l2-mediabus.h>
+
+struct device_node;
+
+struct v4l2_of_mipi_csi2 {
+	unsigned char data_lanes[4];
+	unsigned char clock_lane;
+	unsigned short num_data_lanes;
+};
+
+struct v4l2_of_endpoint {
+	unsigned int port;
+	unsigned int addr;
+	struct list_head head;
+	const struct device_node *local_node;
+	const __be32 *remote;
+	enum v4l2_mbus_type mbus_type;
+	unsigned int mbus_flags;
+	union {
+		struct {
+			unsigned char bus_width;
+			unsigned char data_shift;
+		} parallel;
+		struct v4l2_of_mipi_csi2 mipi_csi_2;
+	};
+};
+
+#ifdef CONFIG_OF
+int v4l2_of_parse_mipi_csi2(const struct device_node *node,
+			    struct v4l2_of_endpoint *endpoint);
+void v4l2_of_parse_parallel_bus(const struct device_node *node,
+				struct v4l2_of_endpoint *endpoint);
+void v4l2_of_parse_endpoint(const struct device_node *node,
+			    struct v4l2_of_endpoint *link);
+struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
+					struct device_node *previous);
+struct device_node *v4l2_of_get_remote_port_parent(
+					const struct device_node *node);
+#else /* CONFIG_OF */
+
+static inline int v4l2_of_parse_endpoint(const struct device_node *node,
+					struct v4l2_of_endpoint *link)
+{
+	return -ENOSYS;
+}
+
+static inline struct device_node *v4l2_of_get_next_endpoint(
+					const struct device_node *parent,
+					struct device_node *previous)
+{
+	return NULL;
+}
+
+static inline struct device_node *v4l2_of_get_remote_endpoint(
+					const struct device_node *node)
+{
+	return NULL;
+}
+
+#endif /* CONFIG_OF */
+
+#endif /* _V4L2_OF_H */
--
1.7.9.5


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

* Re: [PATCH RFC v3 02/15] [media] Add a V4L2 OF parser
  2013-01-03 17:09       ` Sylwester Nawrocki
  (?)
@ 2013-01-18 15:48       ` Sylwester Nawrocki
  2013-01-18 19:02         ` Hans Verkuil
  -1 siblings, 1 reply; 31+ messages in thread
From: Sylwester Nawrocki @ 2013-01-18 15:48 UTC (permalink / raw)
  To: laurent.pinchart, Hans Verkuil, Mauro Carvalho Chehab
  Cc: Sylwester Nawrocki, linux-media, g.liakhovetski, grant.likely,
	rob.herring, thomas.abraham, t.figa, sw0312.kim, kyungmin.park,
	devicetree-discuss

On 01/03/2013 06:09 PM, Sylwester Nawrocki wrote:
> From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> 
> Add a V4L2 OF parser, implementing bindings documented in
> Documentation/devicetree/bindings/media/video-interfaces.txt.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

Hans, Laurent, Mauro, would you have any comments on this ?
Anything that needs to be changed/improved ?

I'd like to send a pull request including this patch, together
with an updated patch series [1] (v3) adding initial device tree
support to the Exynos camera subsystem drivers.

[1] http://www.spinics.net/lists/linux-media/msg57947.html

Thanks,
Sylwester
> ---
> Changes since v2:
>  - added missing EXPORT_SYMBOL for v4l2_of_parse_mipi_csi2()
>    and v4l2_of_parse_parallel_bus() functions,
>  - include string.h header instead of slab.h.
> 
>  drivers/media/v4l2-core/Makefile  |    3 +
>  drivers/media/v4l2-core/v4l2-of.c |  251 +++++++++++++++++++++++++++++++++++++
>  include/media/v4l2-of.h           |   79 ++++++++++++
>  3 files changed, 333 insertions(+)
>  create mode 100644 drivers/media/v4l2-core/v4l2-of.c
>  create mode 100644 include/media/v4l2-of.h
> 
> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> index c2d61d4..00f64d6 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -9,6 +9,9 @@ videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
>  ifeq ($(CONFIG_COMPAT),y)
>    videodev-objs += v4l2-compat-ioctl32.o
>  endif
> +ifeq ($(CONFIG_OF),y)
> +  videodev-objs += v4l2-of.o
> +endif
> 
>  obj-$(CONFIG_VIDEO_DEV) += videodev.o v4l2-int-device.o
>  obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
> diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
> new file mode 100644
> index 0000000..483245c
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -0,0 +1,251 @@
> +/*
> + * V4L2 OF binding parsing library
> + *
> + * Copyright (C) 2012 Renesas Electronics Corp.
> + * Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +
> +#include <media/v4l2-of.h>
> +
> +/**
> + * v4l2_of_parse_mipi_csi2() - parse MIPI CSI-2 bus properties
> + * @node: pointer to endpoint device_node
> + * @endpoint: pointer to v4l2_of_endpoint data structure
> + *
> + * Return: 0 on success or negative error value otherwise.
> + */
> +int v4l2_of_parse_mipi_csi2(const struct device_node *node,
> +			    struct v4l2_of_endpoint *endpoint)
> +{
> +	struct v4l2_of_mipi_csi2 *mipi_csi2 = &endpoint->mipi_csi_2;
> +	u32 data_lanes[ARRAY_SIZE(mipi_csi2->data_lanes)];
> +	struct property *prop;
> +	const __be32 *lane = NULL;
> +	u32 v;
> +	int i = 0;
> +
> +	prop = of_find_property(node, "data-lanes", NULL);
> +	if (!prop)
> +		return -EINVAL;
> +	do {
> +		lane = of_prop_next_u32(prop, lane, &data_lanes[i]);
> +	} while (lane && i++ < ARRAY_SIZE(data_lanes));
> +
> +	mipi_csi2->num_data_lanes = i;
> +	while (i--)
> +		mipi_csi2->data_lanes[i] = data_lanes[i];
> +
> +	if (!of_property_read_u32(node, "clock-lanes", &v))
> +		mipi_csi2->clock_lane = v;
> +
> +	if (of_get_property(node, "clock-noncontinuous", &v))
> +		endpoint->mbus_flags |= V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(v4l2_of_parse_mipi_csi2);
> +
> +/**
> + * v4l2_of_parse_parallel_bus() - parse parallel bus properties
> + * @node: pointer to endpoint device_node
> + * @endpoint: pointer to v4l2_of_endpoint data structure
> + */
> +void v4l2_of_parse_parallel_bus(const struct device_node *node,
> +				struct v4l2_of_endpoint *endpoint)
> +{
> +	unsigned int flags = 0;
> +	u32 v;
> +
> +	if (WARN_ON(!endpoint))
> +		return;
> +
> +	if (!of_property_read_u32(node, "hsync-active", &v))
> +		flags |= v ? V4L2_MBUS_HSYNC_ACTIVE_HIGH :
> +			V4L2_MBUS_HSYNC_ACTIVE_LOW;
> +
> +	if (!of_property_read_u32(node, "vsync-active", &v))
> +		flags |= v ? V4L2_MBUS_VSYNC_ACTIVE_HIGH :
> +			V4L2_MBUS_VSYNC_ACTIVE_LOW;
> +
> +	if (!of_property_read_u32(node, "pclk-sample", &v))
> +		flags |= v ? V4L2_MBUS_PCLK_SAMPLE_RISING :
> +			V4L2_MBUS_PCLK_SAMPLE_FALLING;
> +
> +	if (!of_property_read_u32(node, "field-even-active", &v))
> +		flags |= v ? V4L2_MBUS_FIELD_EVEN_HIGH :
> +			V4L2_MBUS_FIELD_EVEN_LOW;
> +	if (flags)
> +		endpoint->mbus_type = V4L2_MBUS_PARALLEL;
> +	else
> +		endpoint->mbus_type = V4L2_MBUS_BT656;
> +
> +	if (!of_property_read_u32(node, "data-active", &v))
> +		flags |= v ? V4L2_MBUS_DATA_ACTIVE_HIGH :
> +			V4L2_MBUS_DATA_ACTIVE_LOW;
> +
> +	if (of_get_property(node, "slave-mode", &v))
> +		flags |= V4L2_MBUS_SLAVE;
> +
> +	if (!of_property_read_u32(node, "bus-width", &v))
> +		endpoint->parallel.bus_width = v;
> +
> +	if (!of_property_read_u32(node, "data-shift", &v))
> +		endpoint->parallel.data_shift = v;
> +
> +	endpoint->mbus_flags = flags;
> +}
> +EXPORT_SYMBOL(v4l2_of_parse_parallel_bus);
> +
> +/**
> + * v4l2_of_parse_endpoint() - parse all endpoint node properties
> + * @node: pointer to endpoint device_node
> + * @endpoint: pointer to v4l2_of_endpoint data structure
> + *
> + * All properties are optional. If none are found, we don't set any flags.
> + * This means the port has a static configuration and no properties have
> + * to be specified explicitly.
> + * If any properties that identify the bus as parallel are found and
> + * slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if we recognise
> + * the bus as serial CSI-2 and clock-noncontinuous isn't set, we set the
> + * V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag.
> + * The caller should hold a reference to @node.
> + */
> +void v4l2_of_parse_endpoint(const struct device_node *node,
> +			    struct v4l2_of_endpoint *endpoint)
> +{
> +	const struct device_node *port_node = of_get_parent(node);
> +	bool data_lanes_present = false;
> +
> +	memset(endpoint, 0, sizeof(*endpoint));
> +
> +	endpoint->local_node = node;
> +
> +	/* Doesn't matter, whether the below two calls succeed */
> +	of_property_read_u32(port_node, "reg", &endpoint->port);
> +	of_property_read_u32(node, "reg", &endpoint->addr);
> +
> +	v4l2_of_parse_parallel_bus(node, endpoint);
> +
> +	/* If any parallel bus properties have been found, skip serial ones */
> +	if (endpoint->parallel.bus_width || endpoint->parallel.data_shift ||
> +	    endpoint->mbus_flags) {
> +		/* Default parallel bus-master */
> +		if (!(endpoint->mbus_flags & V4L2_MBUS_SLAVE))
> +			endpoint->mbus_flags |= V4L2_MBUS_MASTER;
> +		return;
> +	}
> +
> +	endpoint->mbus_type = V4L2_MBUS_CSI2;
> +
> +	if (!v4l2_of_parse_mipi_csi2(node, endpoint))
> +		data_lanes_present = true;
> +
> +	if ((endpoint->mipi_csi_2.clock_lane || data_lanes_present) &&
> +	    !(endpoint->mbus_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK)) {
> +		/* Default CSI-2: continuous clock */
> +		endpoint->mbus_flags |= V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> +	}
> +}
> +EXPORT_SYMBOL(v4l2_of_parse_endpoint);
> +
> +/*
> + * Return a refcounted next "endpoint" DT node. Contrary to the common OF
> + * practice, we do not drop the reference to previous, users have to do it
> + * themselves, when they're done with the node.
> + */
> +struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
> +					struct device_node *previous)
> +{
> +	struct device_node *child, *port;
> +
> +	if (!parent)
> +		return NULL;
> +
> +	if (!previous) {
> +		/*
> +		 * If this is the first call, we have to find a port within this
> +		 * node
> +		 */
> +		for_each_child_of_node(parent, port) {
> +			if (!of_node_cmp(port->name, "port"))
> +				break;
> +		}
> +		if (port) {
> +			/* Found a port, get a link */
> +			child = of_get_next_child(port, NULL);
> +			of_node_put(port);
> +		} else {
> +			child = NULL;
> +		}
> +		if (!child)
> +			pr_err("%s(): Invalid DT: %s has no link children!\n",
> +			       __func__, parent->name);
> +	} else {
> +		port = of_get_parent(previous);
> +		if (!port)
> +			/* Hm, has someone given us the root node?... */
> +			return NULL;
> +
> +		/* Avoid dropping previous refcount to 0 */
> +		of_node_get(previous);
> +		child = of_get_next_child(port, previous);
> +		if (child) {
> +			of_node_put(port);
> +			return child;
> +		}
> +
> +		/* No more links under this port, try the next one */
> +		do {
> +			port = of_get_next_child(parent, port);
> +			if (!port)
> +				return NULL;
> +		} while (of_node_cmp(port->name, "port"));
> +
> +		/* Pick up the first link on this port */
> +		child = of_get_next_child(port, NULL);
> +		of_node_put(port);
> +	}
> +
> +	return child;
> +}
> +EXPORT_SYMBOL(v4l2_of_get_next_endpoint);
> +
> +/**
> + * v4l2_of_get_remote_port_parent() - get remote port's parent node
> + * @node: pointer to local endpoint device_node
> + *
> + * Return: Remote device node associated with remote endpoint node linked
> + *	   to @node. Use of_node_put() on it when done.
> + */
> +struct device_node *v4l2_of_get_remote_port_parent(
> +			       const struct device_node *node)
> +{
> +	struct device_node *re, *tmp;
> +
> +	/* Get remote endpoint DT node. */
> +	re = of_parse_phandle(node, "remote-endpoint", 0);
> +	if (!re)
> +		return NULL;
> +
> +	/* Remote port. */
> +	tmp = of_get_parent(re);
> +	of_node_put(re);
> +	if (!tmp)
> +		return NULL;
> +
> +	/* Remote device node. */
> +	re = of_get_parent(tmp);
> +	of_node_put(tmp);
> +
> +	return re;
> +}
> +EXPORT_SYMBOL(v4l2_of_get_remote_port_parent);
> diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
> new file mode 100644
> index 0000000..1aba3b3
> --- /dev/null
> +++ b/include/media/v4l2-of.h
> @@ -0,0 +1,79 @@
> +/*
> + * V4L2 OF binding parsing library
> + *
> + * Copyright (C) 2012 Renesas Electronics Corp.
> + * Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + */
> +#ifndef _V4L2_OF_H
> +#define _V4L2_OF_H
> +
> +#include <linux/list.h>
> +#include <linux/types.h>
> +#include <linux/errno.h>
> +
> +#include <media/v4l2-mediabus.h>
> +
> +struct device_node;
> +
> +struct v4l2_of_mipi_csi2 {
> +	unsigned char data_lanes[4];
> +	unsigned char clock_lane;
> +	unsigned short num_data_lanes;
> +};
> +
> +struct v4l2_of_endpoint {
> +	unsigned int port;
> +	unsigned int addr;
> +	struct list_head head;
> +	const struct device_node *local_node;
> +	const __be32 *remote;
> +	enum v4l2_mbus_type mbus_type;
> +	unsigned int mbus_flags;
> +	union {
> +		struct {
> +			unsigned char bus_width;
> +			unsigned char data_shift;
> +		} parallel;
> +		struct v4l2_of_mipi_csi2 mipi_csi_2;
> +	};
> +};
> +
> +#ifdef CONFIG_OF
> +int v4l2_of_parse_mipi_csi2(const struct device_node *node,
> +			    struct v4l2_of_endpoint *endpoint);
> +void v4l2_of_parse_parallel_bus(const struct device_node *node,
> +				struct v4l2_of_endpoint *endpoint);
> +void v4l2_of_parse_endpoint(const struct device_node *node,
> +			    struct v4l2_of_endpoint *link);
> +struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
> +					struct device_node *previous);
> +struct device_node *v4l2_of_get_remote_port_parent(
> +					const struct device_node *node);
> +#else /* CONFIG_OF */
> +
> +static inline int v4l2_of_parse_endpoint(const struct device_node *node,
> +					struct v4l2_of_endpoint *link)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline struct device_node *v4l2_of_get_next_endpoint(
> +					const struct device_node *parent,
> +					struct device_node *previous)
> +{
> +	return NULL;
> +}
> +
> +static inline struct device_node *v4l2_of_get_remote_endpoint(
> +					const struct device_node *node)
> +{
> +	return NULL;
> +}
> +
> +#endif /* CONFIG_OF */
> +
> +#endif /* _V4L2_OF_H */
> --
> 1.7.9.5

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

* Re: [PATCH RFC v3 02/15] [media] Add a V4L2 OF parser
  2013-01-18 15:48       ` Sylwester Nawrocki
@ 2013-01-18 19:02         ` Hans Verkuil
  0 siblings, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2013-01-18 19:02 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: laurent.pinchart, Mauro Carvalho Chehab, linux-media,
	g.liakhovetski, grant.likely, rob.herring, thomas.abraham,
	t.figa, sw0312.kim, kyungmin.park, devicetree-discuss

On Fri January 18 2013 16:48:43 Sylwester Nawrocki wrote:
> On 01/03/2013 06:09 PM, Sylwester Nawrocki wrote:
> > From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > 
> > Add a V4L2 OF parser, implementing bindings documented in
> > Documentation/devicetree/bindings/media/video-interfaces.txt.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> 
> Hans, Laurent, Mauro, would you have any comments on this ?
> Anything that needs to be changed/improved ?

I'll review this on Monday.

Regards,

	Hans

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

* Re: [PATCH RFC v3 01/15] [media] Add common video interfaces OF bindings documentation
  2013-01-03 17:03   ` [PATCH RFC v3 " Sylwester Nawrocki
@ 2013-01-21 10:31     ` Hans Verkuil
  2013-01-23 10:21       ` Sylwester Nawrocki
  0 siblings, 1 reply; 31+ messages in thread
From: Hans Verkuil @ 2013-01-21 10:31 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-media, g.liakhovetski, laurent.pinchart, grant.likely,
	rob.herring, thomas.abraham, t.figa, sw0312.kim, kyungmin.park,
	devicetree-discuss

On Thu January 3 2013 18:03:49 Sylwester Nawrocki wrote:
> From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> 
> This patch adds a document describing common OF bindings for video
> capture, output and video processing devices. It is curently mainly
> focused on video capture devices, with data busses defined by
> standards like ITU-R BT.656 or MIPI-CSI2.
> It also documents a method of describing data links between devices.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>
> Acked-by: Rob Herring <rob.herring@calxeda.com>

As promised, here is my review:

> ---
> 
> Changes since v2:
> 
>  - corrected pclk-sample property definition,
>  - s/buses/busses,
>  - whitespace changes.
> 
>  .../devicetree/bindings/media/video-interfaces.txt |  199 ++++++++++++++++++++
>  1 file changed, 199 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/video-interfaces.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
> new file mode 100644
> index 0000000..9e9e95a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -0,1 +1,199 @@
> +Common bindings for video data receiver and transmitter interfaces
> +
> +General concept
> +---------------
> +
> +Video data pipelines usually consist of external devices, e.g. camera sensors,
> +controlled over an I2C, SPI or UART bus, and SoC internal IP blocks, including
> +video DMA engines and video data processors.
> +
> +SoC internal blocks are described by DT nodes, placed similarly to other SoC
> +blocks.  External devices are represented as child nodes of their respective
> +bus controller nodes, e.g. I2C.
> +
> +Data interfaces on all video devices are described by their child 'port' nodes.
> +Configuration of a port depends on other devices participating in the data
> +transfer and is described by 'endpoint' subnodes.
> +
> +dev {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	port@0 {
> +		endpoint@0 { ... };
> +		endpoint@1 { ... };
> +	};
> +	port@1 { ... };
> +};
> +
> +If a port can be configured to work with more than one other device on the same
> +bus, an 'endpoint' child node must be provided for each of them.  If more than
> +one port is present in a device node or there is more than one endpoint at a
> +port, a common scheme, using '#address-cells', '#size-cells' and 'reg' properties
> +is used.
> +
> +Two 'endpoint' nodes are linked with each other through their 'remote-endpoint'
> +phandles.  An endpoint subnode of a device contains all properties needed for
> +configuration of this device for data exchange with the other device.  In most
> +cases properties at the peer 'endpoint' nodes will be identical, however
> +they might need to be different when there is any signal modifications on the
> +bus between two devices, e.g. there are logic signal inverters on the lines.
> +
> +Required properties
> +-------------------
> +
> +If there is more than one 'port' or more than one 'endpoint' node following
> +properties are required in relevant parent node:
> +
> +- #address-cells : number of cells required to define port number, should be 1.
> +- #size-cells    : should be zero.
> +
> +Optional endpoint properties
> +----------------------------
> +
> +- remote-endpoint: phandle to an 'endpoint' subnode of the other device node.
> +- slave-mode: a boolean property, run the link in slave mode. Default is master
> +  mode.
> +- bus-width: number of data lines, valid for parallel busses.
> +- data-shift: on parallel data busses, if bus-width is used to specify the
> +  number of data lines, data-shift can be used to specify which data lines are
> +  used, e.g. "bus-width=<10>; data-shift=<2>;" means, that lines 9:2 are used.
> +- hsync-active: active state of HSYNC signal, 0/1 for LOW/HIGH respectively.
> +- vsync-active: active state of VSYNC signal, 0/1 for LOW/HIGH respectively.
> +  Note, that if HSYNC and VSYNC polarities are not specified, embedded
> +  synchronization may be required, where supported.
> +- data-active: similar to HSYNC and VSYNC, specifies data line polarity.
> +- field-even-active: field signal level during the even field data transmission.
> +- pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock
> +  signal.
> +- data-lanes: an array of physical data lane indexes. Position of an entry
> +  determines logical lane number, while the value of an entry indicates physical
> +  lane, e.g. for 2-lane MIPI CSI-2 bus we could have "data-lanes = <1>, <2>;",
> +  assuming the clock lane is on hardware lane 0. This property is valid for
> +  serial busses only (e.g. MIPI CSI-2).
> +- clock-lanes: a number of physical lane used as a clock lane.

This doesn't parse. Do you mean:

"a number of physical lanes used as clock lanes."?

> +- clock-noncontinuous: a boolean property to allow MIPI CSI-2 non-continuous
> +  clock mode.
> +
> +Example
> +-------
> +
> +The below example snippet describes two data pipelines.  ov772x and imx074 are

s/below example snippet/example snippet below/

> +camera sensors with parallel and serial (MIPI CSI-2) video bus respectively.

s/with/with a/

> +Both sensors are on I2C control bus corresponding to i2c0 controller node.

s/on/on the/
s/to/to the/

> +ov772x sensor is linked directly to the ceu0 video host interface.  imx074 is
> +linked to ceu0 through MIPI CSI-2 receiver (csi2). ceu0 has a (single) DMA

s/through/through the/

> +engine writing captured data to memory.  ceu0 node has single 'port' node which

s/single/a single/

> +indicates at any time only one of following data pipeline can be active:

s/at/that at/
s/one of/one of the/
s/pipeline/pipelines/

> +ov772x -> ceu0 or imx074 -> csi2 -> ceu0.
> +
> +	ceu0: ceu@0xfe910000 {
> +		compatible = "renesas,sh-mobile-ceu";
> +		reg = <0xfe910000 0xa0>;
> +		interrupts = <0x880>;
> +
> +		mclk: master_clock {
> +			compatible = "renesas,ceu-clock";
> +			#clock-cells = <1>;
> +			clock-frequency = <50000000>;	/* Max clock frequency */
> +			clock-output-names = "mclk";
> +		};
> +
> +		port {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			ceu0_1: endpoint@1 {
> +				reg = <1>;		/* Local endpoint # */
> +				remote = <&ov772x_1_1>;	/* Remote phandle */
> +				bus-width = <8>;	/* Used data lines */
> +				data-shift = <0>;	/* Lines 7:0 are used */
> +
> +				/* If hsync-active/vsync-active are missing,
> +				   embedded bt.605 sync is used */
> +				hsync-active = <1>;	/* Active high */
> +				vsync-active = <1>;	/* Active high */
> +				data-active = <1>;	/* Active high */
> +				pclk-sample = <1>;	/* Rising */
> +			};
> +
> +			ceu0_0: endpoint@0 {
> +				reg = <0>;
> +				remote = <&csi2_2>;
> +				immutable;
> +			};
> +		};
> +	};
> +
> +	i2c0: i2c@0xfff20000 {
> +		...
> +		ov772x_1: camera@0x21 {
> +			compatible = "omnivision,ov772x";
> +			reg = <0x21>;
> +			vddio-supply = <&regulator1>;
> +			vddcore-supply = <&regulator2>;
> +
> +			clock-frequency = <20000000>;
> +			clocks = <&mclk 0>;
> +			clock-names = "xclk";
> +
> +			port {
> +				/* With 1 endpoint per port no need in addresses. */
> +				ov772x_1_1: endpoint {
> +					bus-width = <8>;
> +					remote-endpoint = <&ceu0_1>;
> +					hsync-active = <1>;
> +					vsync-active = <0>; /* Who came up with an
> +							       inverter here ?... */
> +					data-active = <1>;
> +					pclk-sample = <1>;
> +				};
> +			};
> +		};
> +
> +		imx074: camera@0x1a {
> +			compatible = "sony,imx074";
> +			reg = <0x1a>;
> +			vddio-supply = <&regulator1>;
> +			vddcore-supply = <&regulator2>;
> +
> +			clock-frequency = <30000000>;	/* Shared clock with ov772x_1 */
> +			clocks = <&mclk 0>;
> +			clock-names = "sysclk";		/* Assuming this is the
> +							   name in the datasheet */
> +			port {
> +				imx074_1: endpoint {
> +					clock-lanes = <0>;
> +					data-lanes = <1>, <2>;
> +					remote-endpoint = <&csi2_1>;
> +				};
> +			};
> +		};
> +	};
> +
> +	csi2: csi2@0xffc90000 {
> +		compatible = "renesas,sh-mobile-csi2";
> +		reg = <0xffc90000 0x1000>;
> +		interrupts = <0x17a0>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port@1 {
> +			compatible = "renesas,csi2c";	/* One of CSI2I and CSI2C. */
> +			reg = <1>;			/* CSI-2 PHY #1 of 2: PHY_S,
> +							   PHY_M has port address 0,
> +							   is unused. */
> +			csi2_1: endpoint {
> +				clock-lanes = <0>;
> +				data-lanes = <2>, <1>;
> +				remote-endpoint = <&imx074_1>;
> +			};
> +		};
> +		port@2 {
> +			reg = <2>;			/* port 2: link to the CEU */
> +
> +			csi2_2: endpoint {
> +				immutable;
> +				remote-endpoint = <&ceu0_0>;
> +			};
> +		};
> +	};

Regards,

	Hans

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

* Re: [PATCH RFC v3 02/15] [media] Add a V4L2 OF parser
  2013-01-03 17:09       ` Sylwester Nawrocki
  (?)
  (?)
@ 2013-01-21 11:35       ` Hans Verkuil
  2013-01-23 10:44         ` Sylwester Nawrocki
  -1 siblings, 1 reply; 31+ messages in thread
From: Hans Verkuil @ 2013-01-21 11:35 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-media, g.liakhovetski, laurent.pinchart, grant.likely,
	rob.herring, thomas.abraham, t.figa, sw0312.kim, kyungmin.park,
	devicetree-discuss

Hi Sylwester,

Just two comments...

On Thu January 3 2013 18:09:22 Sylwester Nawrocki wrote:
> From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> 
> Add a V4L2 OF parser, implementing bindings documented in
> Documentation/devicetree/bindings/media/video-interfaces.txt.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> Changes since v2:
>  - added missing EXPORT_SYMBOL for v4l2_of_parse_mipi_csi2()
>    and v4l2_of_parse_parallel_bus() functions,
>  - include string.h header instead of slab.h.
> 
>  drivers/media/v4l2-core/Makefile  |    3 +
>  drivers/media/v4l2-core/v4l2-of.c |  251 +++++++++++++++++++++++++++++++++++++
>  include/media/v4l2-of.h           |   79 ++++++++++++
>  3 files changed, 333 insertions(+)
>  create mode 100644 drivers/media/v4l2-core/v4l2-of.c
>  create mode 100644 include/media/v4l2-of.h
> 
> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> index c2d61d4..00f64d6 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -9,6 +9,9 @@ videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
>  ifeq ($(CONFIG_COMPAT),y)
>    videodev-objs += v4l2-compat-ioctl32.o
>  endif
> +ifeq ($(CONFIG_OF),y)
> +  videodev-objs += v4l2-of.o
> +endif
> 
>  obj-$(CONFIG_VIDEO_DEV) += videodev.o v4l2-int-device.o
>  obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
> diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
> new file mode 100644
> index 0000000..483245c
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -0,0 +1,251 @@
> +/*
> + * V4L2 OF binding parsing library
> + *
> + * Copyright (C) 2012 Renesas Electronics Corp.
> + * Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +
> +#include <media/v4l2-of.h>
> +
> +/**
> + * v4l2_of_parse_mipi_csi2() - parse MIPI CSI-2 bus properties
> + * @node: pointer to endpoint device_node
> + * @endpoint: pointer to v4l2_of_endpoint data structure
> + *
> + * Return: 0 on success or negative error value otherwise.
> + */
> +int v4l2_of_parse_mipi_csi2(const struct device_node *node,
> +			    struct v4l2_of_endpoint *endpoint)
> +{
> +	struct v4l2_of_mipi_csi2 *mipi_csi2 = &endpoint->mipi_csi_2;
> +	u32 data_lanes[ARRAY_SIZE(mipi_csi2->data_lanes)];
> +	struct property *prop;
> +	const __be32 *lane = NULL;
> +	u32 v;
> +	int i = 0;
> +
> +	prop = of_find_property(node, "data-lanes", NULL);
> +	if (!prop)
> +		return -EINVAL;
> +	do {
> +		lane = of_prop_next_u32(prop, lane, &data_lanes[i]);
> +	} while (lane && i++ < ARRAY_SIZE(data_lanes));
> +
> +	mipi_csi2->num_data_lanes = i;
> +	while (i--)
> +		mipi_csi2->data_lanes[i] = data_lanes[i];
> +
> +	if (!of_property_read_u32(node, "clock-lanes", &v))
> +		mipi_csi2->clock_lane = v;

Hmm, the property name is 'clock-lanes', but only one lane is obtained here.

Why is the property name plural in that case?

> +
> +	if (of_get_property(node, "clock-noncontinuous", &v))
> +		endpoint->mbus_flags |= V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(v4l2_of_parse_mipi_csi2);
> +
> +/**
> + * v4l2_of_parse_parallel_bus() - parse parallel bus properties
> + * @node: pointer to endpoint device_node
> + * @endpoint: pointer to v4l2_of_endpoint data structure
> + */
> +void v4l2_of_parse_parallel_bus(const struct device_node *node,
> +				struct v4l2_of_endpoint *endpoint)
> +{
> +	unsigned int flags = 0;
> +	u32 v;
> +
> +	if (WARN_ON(!endpoint))
> +		return;
> +
> +	if (!of_property_read_u32(node, "hsync-active", &v))
> +		flags |= v ? V4L2_MBUS_HSYNC_ACTIVE_HIGH :
> +			V4L2_MBUS_HSYNC_ACTIVE_LOW;
> +
> +	if (!of_property_read_u32(node, "vsync-active", &v))
> +		flags |= v ? V4L2_MBUS_VSYNC_ACTIVE_HIGH :
> +			V4L2_MBUS_VSYNC_ACTIVE_LOW;
> +
> +	if (!of_property_read_u32(node, "pclk-sample", &v))
> +		flags |= v ? V4L2_MBUS_PCLK_SAMPLE_RISING :
> +			V4L2_MBUS_PCLK_SAMPLE_FALLING;
> +
> +	if (!of_property_read_u32(node, "field-even-active", &v))
> +		flags |= v ? V4L2_MBUS_FIELD_EVEN_HIGH :
> +			V4L2_MBUS_FIELD_EVEN_LOW;
> +	if (flags)
> +		endpoint->mbus_type = V4L2_MBUS_PARALLEL;
> +	else
> +		endpoint->mbus_type = V4L2_MBUS_BT656;
> +
> +	if (!of_property_read_u32(node, "data-active", &v))
> +		flags |= v ? V4L2_MBUS_DATA_ACTIVE_HIGH :
> +			V4L2_MBUS_DATA_ACTIVE_LOW;
> +
> +	if (of_get_property(node, "slave-mode", &v))
> +		flags |= V4L2_MBUS_SLAVE;
> +
> +	if (!of_property_read_u32(node, "bus-width", &v))
> +		endpoint->parallel.bus_width = v;
> +
> +	if (!of_property_read_u32(node, "data-shift", &v))
> +		endpoint->parallel.data_shift = v;
> +
> +	endpoint->mbus_flags = flags;
> +}
> +EXPORT_SYMBOL(v4l2_of_parse_parallel_bus);
> +
> +/**
> + * v4l2_of_parse_endpoint() - parse all endpoint node properties
> + * @node: pointer to endpoint device_node
> + * @endpoint: pointer to v4l2_of_endpoint data structure
> + *
> + * All properties are optional. If none are found, we don't set any flags.
> + * This means the port has a static configuration and no properties have
> + * to be specified explicitly.
> + * If any properties that identify the bus as parallel are found and
> + * slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if we recognise
> + * the bus as serial CSI-2 and clock-noncontinuous isn't set, we set the
> + * V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag.
> + * The caller should hold a reference to @node.
> + */
> +void v4l2_of_parse_endpoint(const struct device_node *node,
> +			    struct v4l2_of_endpoint *endpoint)
> +{
> +	const struct device_node *port_node = of_get_parent(node);
> +	bool data_lanes_present = false;
> +
> +	memset(endpoint, 0, sizeof(*endpoint));
> +
> +	endpoint->local_node = node;
> +
> +	/* Doesn't matter, whether the below two calls succeed */

'It doesn't matter whether the two calls below succeed. If they don't
then the default value 0 is used.'

At least, that's how I understand the code.

> +	of_property_read_u32(port_node, "reg", &endpoint->port);
> +	of_property_read_u32(node, "reg", &endpoint->addr);
> +
> +	v4l2_of_parse_parallel_bus(node, endpoint);
> +
> +	/* If any parallel bus properties have been found, skip serial ones */
> +	if (endpoint->parallel.bus_width || endpoint->parallel.data_shift ||
> +	    endpoint->mbus_flags) {
> +		/* Default parallel bus-master */
> +		if (!(endpoint->mbus_flags & V4L2_MBUS_SLAVE))
> +			endpoint->mbus_flags |= V4L2_MBUS_MASTER;
> +		return;
> +	}
> +
> +	endpoint->mbus_type = V4L2_MBUS_CSI2;
> +
> +	if (!v4l2_of_parse_mipi_csi2(node, endpoint))
> +		data_lanes_present = true;
> +
> +	if ((endpoint->mipi_csi_2.clock_lane || data_lanes_present) &&
> +	    !(endpoint->mbus_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK)) {
> +		/* Default CSI-2: continuous clock */
> +		endpoint->mbus_flags |= V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> +	}
> +}
> +EXPORT_SYMBOL(v4l2_of_parse_endpoint);
> +
> +/*
> + * Return a refcounted next "endpoint" DT node. Contrary to the common OF
> + * practice, we do not drop the reference to previous, users have to do it
> + * themselves, when they're done with the node.
> + */
> +struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
> +					struct device_node *previous)
> +{
> +	struct device_node *child, *port;
> +
> +	if (!parent)
> +		return NULL;
> +
> +	if (!previous) {
> +		/*
> +		 * If this is the first call, we have to find a port within this
> +		 * node
> +		 */
> +		for_each_child_of_node(parent, port) {
> +			if (!of_node_cmp(port->name, "port"))
> +				break;
> +		}
> +		if (port) {
> +			/* Found a port, get a link */
> +			child = of_get_next_child(port, NULL);
> +			of_node_put(port);
> +		} else {
> +			child = NULL;
> +		}
> +		if (!child)
> +			pr_err("%s(): Invalid DT: %s has no link children!\n",
> +			       __func__, parent->name);
> +	} else {
> +		port = of_get_parent(previous);
> +		if (!port)
> +			/* Hm, has someone given us the root node?... */
> +			return NULL;
> +
> +		/* Avoid dropping previous refcount to 0 */
> +		of_node_get(previous);
> +		child = of_get_next_child(port, previous);
> +		if (child) {
> +			of_node_put(port);
> +			return child;
> +		}
> +
> +		/* No more links under this port, try the next one */
> +		do {
> +			port = of_get_next_child(parent, port);
> +			if (!port)
> +				return NULL;
> +		} while (of_node_cmp(port->name, "port"));
> +
> +		/* Pick up the first link on this port */
> +		child = of_get_next_child(port, NULL);
> +		of_node_put(port);
> +	}
> +
> +	return child;
> +}
> +EXPORT_SYMBOL(v4l2_of_get_next_endpoint);
> +
> +/**
> + * v4l2_of_get_remote_port_parent() - get remote port's parent node
> + * @node: pointer to local endpoint device_node
> + *
> + * Return: Remote device node associated with remote endpoint node linked
> + *	   to @node. Use of_node_put() on it when done.
> + */
> +struct device_node *v4l2_of_get_remote_port_parent(
> +			       const struct device_node *node)
> +{
> +	struct device_node *re, *tmp;
> +
> +	/* Get remote endpoint DT node. */
> +	re = of_parse_phandle(node, "remote-endpoint", 0);
> +	if (!re)
> +		return NULL;
> +
> +	/* Remote port. */
> +	tmp = of_get_parent(re);
> +	of_node_put(re);
> +	if (!tmp)
> +		return NULL;
> +
> +	/* Remote device node. */
> +	re = of_get_parent(tmp);
> +	of_node_put(tmp);
> +
> +	return re;
> +}
> +EXPORT_SYMBOL(v4l2_of_get_remote_port_parent);
> diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
> new file mode 100644
> index 0000000..1aba3b3
> --- /dev/null
> +++ b/include/media/v4l2-of.h
> @@ -0,0 +1,79 @@
> +/*
> + * V4L2 OF binding parsing library
> + *
> + * Copyright (C) 2012 Renesas Electronics Corp.
> + * Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + */
> +#ifndef _V4L2_OF_H
> +#define _V4L2_OF_H
> +
> +#include <linux/list.h>
> +#include <linux/types.h>
> +#include <linux/errno.h>
> +
> +#include <media/v4l2-mediabus.h>
> +
> +struct device_node;
> +
> +struct v4l2_of_mipi_csi2 {
> +	unsigned char data_lanes[4];
> +	unsigned char clock_lane;
> +	unsigned short num_data_lanes;
> +};
> +
> +struct v4l2_of_endpoint {
> +	unsigned int port;
> +	unsigned int addr;
> +	struct list_head head;
> +	const struct device_node *local_node;
> +	const __be32 *remote;
> +	enum v4l2_mbus_type mbus_type;
> +	unsigned int mbus_flags;
> +	union {
> +		struct {
> +			unsigned char bus_width;
> +			unsigned char data_shift;
> +		} parallel;
> +		struct v4l2_of_mipi_csi2 mipi_csi_2;
> +	};
> +};
> +
> +#ifdef CONFIG_OF
> +int v4l2_of_parse_mipi_csi2(const struct device_node *node,
> +			    struct v4l2_of_endpoint *endpoint);
> +void v4l2_of_parse_parallel_bus(const struct device_node *node,
> +				struct v4l2_of_endpoint *endpoint);
> +void v4l2_of_parse_endpoint(const struct device_node *node,
> +			    struct v4l2_of_endpoint *link);
> +struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
> +					struct device_node *previous);
> +struct device_node *v4l2_of_get_remote_port_parent(
> +					const struct device_node *node);
> +#else /* CONFIG_OF */
> +
> +static inline int v4l2_of_parse_endpoint(const struct device_node *node,
> +					struct v4l2_of_endpoint *link)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline struct device_node *v4l2_of_get_next_endpoint(
> +					const struct device_node *parent,
> +					struct device_node *previous)
> +{
> +	return NULL;
> +}
> +
> +static inline struct device_node *v4l2_of_get_remote_endpoint(
> +					const struct device_node *node)
> +{
> +	return NULL;
> +}
> +
> +#endif /* CONFIG_OF */
> +
> +#endif /* _V4L2_OF_H */

Regards,

	Hans

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

* Re: [PATCH RFC v3 01/15] [media] Add common video interfaces OF bindings documentation
  2013-01-21 10:31     ` Hans Verkuil
@ 2013-01-23 10:21       ` Sylwester Nawrocki
  2013-01-23 12:59         ` Hans Verkuil
  0 siblings, 1 reply; 31+ messages in thread
From: Sylwester Nawrocki @ 2013-01-23 10:21 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, g.liakhovetski, laurent.pinchart, grant.likely,
	rob.herring, thomas.abraham, t.figa, sw0312.kim, kyungmin.park,
	devicetree-discuss

Hi Hans,

On 01/21/2013 11:31 AM, Hans Verkuil wrote:
[...]
>> +Required properties
>> +-------------------
>> +
>> +If there is more than one 'port' or more than one 'endpoint' node following
>> +properties are required in relevant parent node:
>> +
>> +- #address-cells : number of cells required to define port number, should be 1.
>> +- #size-cells    : should be zero.
>> +
>> +Optional endpoint properties
>> +----------------------------
>> +
>> +- remote-endpoint: phandle to an 'endpoint' subnode of the other device node.
>> +- slave-mode: a boolean property, run the link in slave mode. Default is master
>> +  mode.
>> +- bus-width: number of data lines, valid for parallel busses.
>> +- data-shift: on parallel data busses, if bus-width is used to specify the
>> +  number of data lines, data-shift can be used to specify which data lines are
>> +  used, e.g. "bus-width=<10>; data-shift=<2>;" means, that lines 9:2 are used.
>> +- hsync-active: active state of HSYNC signal, 0/1 for LOW/HIGH respectively.
>> +- vsync-active: active state of VSYNC signal, 0/1 for LOW/HIGH respectively.
>> +  Note, that if HSYNC and VSYNC polarities are not specified, embedded
>> +  synchronization may be required, where supported.
>> +- data-active: similar to HSYNC and VSYNC, specifies data line polarity.
>> +- field-even-active: field signal level during the even field data transmission.
>> +- pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock
>> +  signal.
>> +- data-lanes: an array of physical data lane indexes. Position of an entry
>> +  determines logical lane number, while the value of an entry indicates physical
>> +  lane, e.g. for 2-lane MIPI CSI-2 bus we could have "data-lanes = <1>, <2>;",
>> +  assuming the clock lane is on hardware lane 0. This property is valid for
>> +  serial busses only (e.g. MIPI CSI-2).
>> +- clock-lanes: a number of physical lane used as a clock lane.
> 
> This doesn't parse. Do you mean:
> 
> "a number of physical lanes used as clock lanes."?

Not really, an index (an array of indexes?) of physical lanes(s) used as clock
lane (s).

Currently there are only use cases for one clock lane (MIPI CSI-2 bus).
I'm not sure what's better, to keep that in singular (clock-lane) or plural
form. The plural form seems more generic. So I'm inclined to define it as:

clock-lanes - similarly to 'data-lanes' property, an array of physical
clock lane indexes. For MIPI CSI-2 bus this array contains only one entry.

Would it be OK like this ?

>> +- clock-noncontinuous: a boolean property to allow MIPI CSI-2 non-continuous
>> +  clock mode.
>> +
>> +Example
>> +-------
>> +
>> +The below example snippet describes two data pipelines.  ov772x and imx074 are
> 
> s/below example snippet/example snippet below/
> 
>> +camera sensors with parallel and serial (MIPI CSI-2) video bus respectively.
> 
> s/with/with a/
> 
>> +Both sensors are on I2C control bus corresponding to i2c0 controller node.
> 
> s/on/on the/
> s/to/to the/
> 
>> +ov772x sensor is linked directly to the ceu0 video host interface.  imx074 is
>> +linked to ceu0 through MIPI CSI-2 receiver (csi2). ceu0 has a (single) DMA
> 
> s/through/through the/
> 
>> +engine writing captured data to memory.  ceu0 node has single 'port' node which
> 
> s/single/a single/
> 
>> +indicates at any time only one of following data pipeline can be active:
> 
> s/at/that at/
> s/one of/one of the/
> s/pipeline/pipelines/

Ok, I'll corect all these. Thanks.

--

Regards,
Sylwester

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

* Re: [PATCH RFC v3 02/15] [media] Add a V4L2 OF parser
  2013-01-21 11:35       ` Hans Verkuil
@ 2013-01-23 10:44         ` Sylwester Nawrocki
  0 siblings, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2013-01-23 10:44 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, g.liakhovetski, laurent.pinchart, grant.likely,
	rob.herring, thomas.abraham, t.figa, sw0312.kim, kyungmin.park,
	devicetree-discuss

On 01/21/2013 12:35 PM, Hans Verkuil wrote:
[...]
>> +/**
>> + * v4l2_of_parse_mipi_csi2() - parse MIPI CSI-2 bus properties
>> + * @node: pointer to endpoint device_node
>> + * @endpoint: pointer to v4l2_of_endpoint data structure
>> + *
>> + * Return: 0 on success or negative error value otherwise.
>> + */
>> +int v4l2_of_parse_mipi_csi2(const struct device_node *node,
>> +			    struct v4l2_of_endpoint *endpoint)
>> +{
>> +	struct v4l2_of_mipi_csi2 *mipi_csi2 = &endpoint->mipi_csi_2;
>> +	u32 data_lanes[ARRAY_SIZE(mipi_csi2->data_lanes)];
>> +	struct property *prop;
>> +	const __be32 *lane = NULL;
>> +	u32 v;
>> +	int i = 0;
>> +
>> +	prop = of_find_property(node, "data-lanes", NULL);
>> +	if (!prop)
>> +		return -EINVAL;
>> +	do {
>> +		lane = of_prop_next_u32(prop, lane, &data_lanes[i]);
>> +	} while (lane && i++ < ARRAY_SIZE(data_lanes));
>> +
>> +	mipi_csi2->num_data_lanes = i;
>> +	while (i--)
>> +		mipi_csi2->data_lanes[i] = data_lanes[i];
>> +
>> +	if (!of_property_read_u32(node, "clock-lanes", &v))
>> +		mipi_csi2->clock_lane = v;
> 
> Hmm, the property name is 'clock-lanes', but only one lane is obtained here.
> 
> Why is the property name plural in that case?

This is how we agreed it with Guennadi, the argumentation was that it is 
more consistent with 'data-lanes'. Also I think it is better to use plural 
form right from the beginning, rather than introducing another 'clock-lanes' 
property along 'clock-lane' when there would be a need to handle busses 
with more than one clock lane in the future.

[...]
>> +/**
>> + * v4l2_of_parse_endpoint() - parse all endpoint node properties
>> + * @node: pointer to endpoint device_node
>> + * @endpoint: pointer to v4l2_of_endpoint data structure
>> + *
>> + * All properties are optional. If none are found, we don't set any flags.
>> + * This means the port has a static configuration and no properties have
>> + * to be specified explicitly.
>> + * If any properties that identify the bus as parallel are found and
>> + * slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if we recognise
>> + * the bus as serial CSI-2 and clock-noncontinuous isn't set, we set the
>> + * V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag.
>> + * The caller should hold a reference to @node.
>> + */
>> +void v4l2_of_parse_endpoint(const struct device_node *node,
>> +			    struct v4l2_of_endpoint *endpoint)
>> +{
>> +	const struct device_node *port_node = of_get_parent(node);
>> +	bool data_lanes_present = false;
>> +
>> +	memset(endpoint, 0, sizeof(*endpoint));
>> +
>> +	endpoint->local_node = node;
>> +
>> +	/* Doesn't matter, whether the below two calls succeed */
> 
> 'It doesn't matter whether the two calls below succeed. If they don't
> then the default value 0 is used.'
> 
> At least, that's how I understand the code.

Yeah, it's more precise this way. I'll amend it, thanks!

>> +	of_property_read_u32(port_node, "reg", &endpoint->port);
>> +	of_property_read_u32(node, "reg", &endpoint->addr);

--

Regards,
Sylwester

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

* Re: [PATCH RFC v3 01/15] [media] Add common video interfaces OF bindings documentation
  2013-01-23 10:21       ` Sylwester Nawrocki
@ 2013-01-23 12:59         ` Hans Verkuil
  0 siblings, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2013-01-23 12:59 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-media, g.liakhovetski, laurent.pinchart, grant.likely,
	rob.herring, thomas.abraham, t.figa, sw0312.kim, kyungmin.park,
	devicetree-discuss

On Wed 23 January 2013 11:21:24 Sylwester Nawrocki wrote:
> Hi Hans,
> 
> On 01/21/2013 11:31 AM, Hans Verkuil wrote:
> [...]
> >> +Required properties
> >> +-------------------
> >> +
> >> +If there is more than one 'port' or more than one 'endpoint' node following
> >> +properties are required in relevant parent node:
> >> +
> >> +- #address-cells : number of cells required to define port number, should be 1.
> >> +- #size-cells    : should be zero.
> >> +
> >> +Optional endpoint properties
> >> +----------------------------
> >> +
> >> +- remote-endpoint: phandle to an 'endpoint' subnode of the other device node.
> >> +- slave-mode: a boolean property, run the link in slave mode. Default is master
> >> +  mode.
> >> +- bus-width: number of data lines, valid for parallel busses.
> >> +- data-shift: on parallel data busses, if bus-width is used to specify the
> >> +  number of data lines, data-shift can be used to specify which data lines are
> >> +  used, e.g. "bus-width=<10>; data-shift=<2>;" means, that lines 9:2 are used.
> >> +- hsync-active: active state of HSYNC signal, 0/1 for LOW/HIGH respectively.
> >> +- vsync-active: active state of VSYNC signal, 0/1 for LOW/HIGH respectively.
> >> +  Note, that if HSYNC and VSYNC polarities are not specified, embedded
> >> +  synchronization may be required, where supported.
> >> +- data-active: similar to HSYNC and VSYNC, specifies data line polarity.
> >> +- field-even-active: field signal level during the even field data transmission.
> >> +- pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock
> >> +  signal.
> >> +- data-lanes: an array of physical data lane indexes. Position of an entry
> >> +  determines logical lane number, while the value of an entry indicates physical
> >> +  lane, e.g. for 2-lane MIPI CSI-2 bus we could have "data-lanes = <1>, <2>;",
> >> +  assuming the clock lane is on hardware lane 0. This property is valid for
> >> +  serial busses only (e.g. MIPI CSI-2).
> >> +- clock-lanes: a number of physical lane used as a clock lane.
> > 
> > This doesn't parse. Do you mean:
> > 
> > "a number of physical lanes used as clock lanes."?
> 
> Not really, an index (an array of indexes?) of physical lanes(s) used as clock
> lane (s).
> 
> Currently there are only use cases for one clock lane (MIPI CSI-2 bus).
> I'm not sure what's better, to keep that in singular (clock-lane) or plural
> form. The plural form seems more generic. So I'm inclined to define it as:
> 
> clock-lanes - similarly to 'data-lanes' property, an array of physical
> clock lane indexes. For MIPI CSI-2 bus this array contains only one entry.
> 
> Would it be OK like this ?

I'd go with this:

- clock-lanes: an array of physical clock lane indexes. Position of an entry
  determines the logical lane number, while the value of an entry indicates
  physical lane, e.g. for a MIPI CSI-2 bus we could have "clock-lanes = <0>;",
  which places the clock lane on hardware lane 0. This property is valid for
  serial busses only (e.g. MIPI CSI-2). Note that for the MIPI CSI-2 bus this
  array contains only one entry.

Regards,

	Hans

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

end of thread, other threads:[~2013-01-23 12:59 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-31 16:02 [PATCH RFC v2 00/15] V4L2 device tree bindings and OF helpers Sylwester Nawrocki
2012-12-31 16:02 ` [PATCH RFC v2 01/15] [media] Add common video interfaces OF bindings documentation Sylwester Nawrocki
2013-01-02 11:31   ` Guennadi Liakhovetski
2013-01-02 21:51     ` Sylwester Nawrocki
2013-01-02 22:01       ` Guennadi Liakhovetski
2013-01-03 17:03   ` [PATCH RFC v3 " Sylwester Nawrocki
2013-01-21 10:31     ` Hans Verkuil
2013-01-23 10:21       ` Sylwester Nawrocki
2013-01-23 12:59         ` Hans Verkuil
2012-12-31 16:03 ` [PATCH RFC v2 02/15] [media] Add a V4L2 OF parser Sylwester Nawrocki
2013-01-02 11:58   ` Guennadi Liakhovetski
2013-01-02 22:11     ` Sylwester Nawrocki
     [not found]   ` <1356969793-27268-3-git-send-email-s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2013-01-03 17:09     ` [PATCH RFC v3 " Sylwester Nawrocki
2013-01-03 17:09       ` Sylwester Nawrocki
2013-01-18 15:48       ` Sylwester Nawrocki
2013-01-18 19:02         ` Hans Verkuil
2013-01-21 11:35       ` Hans Verkuil
2013-01-23 10:44         ` Sylwester Nawrocki
2012-12-31 16:03 ` [PATCH RFC v2 03/15] s5p-csis: Add device tree support Sylwester Nawrocki
2012-12-31 16:03 ` [PATCH RFC v2 04/15] s5p-fimc: Support for FIMC devices instantiated from the device tree Sylwester Nawrocki
2012-12-31 16:03 ` [PATCH RFC v2 05/15] s5p-fimc: Support for FIMC-LITE " Sylwester Nawrocki
2012-12-31 16:03 ` [PATCH RFC v2 06/15] s5p-fimc: Change platform subdevs registration method Sylwester Nawrocki
2012-12-31 16:03 ` [PATCH RFC v2 07/15] s5p-fimc: Support camera media device initialization on DT systems Sylwester Nawrocki
2012-12-31 16:03 ` [PATCH RFC v2 08/15] s5p-fimc: Add device tree based sensors registration Sylwester Nawrocki
2012-12-31 16:03 ` [PATCH RFC v2 09/15] s5p-fimc: Use pinctrl API for camera ports configuration Sylwester Nawrocki
2012-12-31 16:03 ` [PATCH RFC v2 10/15] ARM: EXYNOS4: Add OF_DEV_AUXDATA for FIMC, FIMC-LITE and CSIS Sylwester Nawrocki
2012-12-31 16:03 ` [PATCH RFC v2 11/15] ARM: dts: Add camera node exynos4.dtsi Sylwester Nawrocki
2012-12-31 16:03 ` [PATCH RFC v2 12/15] ARM: dts: Add ISP power domain node for Exynos4x12 Sylwester Nawrocki
2012-12-31 16:03 ` [PATCH RFC v2 13/15] ARM: dts: Add FIMC and MIPI CSIS device nodes " Sylwester Nawrocki
2012-12-31 16:03 ` [PATCH RFC v2 14/15] ARM: dts: Add camera pinctrl nodes for Exynos4x12 SoCs Sylwester Nawrocki
2012-12-31 16:03 ` [PATCH RFC v2 15/15] ARM: dts: Add camera device nodes nodes for PQ board Sylwester Nawrocki

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