[PATCHv3 00/15] atmel-isi/ov7670/ov2640: convert to standalone drivers

[PATCHv3 00/15] atmel-isi/ov7670/ov2640: convert to standalone drivers

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

This patch series converts the soc-camera atmel-isi to a standalone V4L2
driver.

The same is done for the ov7670 and ov2640 sensor drivers: the ov7670 was
used to test the atmel-isi driver. The ov2640 is needed because the em28xx
driver has a soc_camera include dependency. Both ov7670 and ov2640 sensors
have been tested with the atmel-isi driver.

The first 5 patches improve the ov7670 sensor driver, mostly adding modern
features such as DT support.

The next three convert the atmel-isi and move it out of soc_camera.

The following 5 patches convert ov2640 and drop the soc_camera dependency
in em28xx. I have tested that this works with my 'SpeedLink Vicious And
Divine Laplace webcam'.

The last two patches add isi support to the DT: the first for the ov7670
sensor, the second modifies it for the ov2640 sensor.

These two final patches are for demonstration purposes only, I do not plan
on merging them.

Tested with my sama5d3-Xplained board, the ov2640 sensor and two ov7670
sensors: one with and one without reset/pwdn pins. Also tested with my
em28xx-based webcam.

I'd like to get this in for 4.12. Fingers crossed.

Regards,

	Hans

Changes since v2:
- Incorporated Sakari's and Rob's device tree bindings comments.
- ov2640: dropped the reset/power changes. These actually broke the em28xx
  and there was really nothing wrong with it.
- merged the "ov2640: allow use inside em28xx" into patches 10 and 11.
  It really shouldn't have been a separate patch in the first place.
- rebased on top of 4.11-rc1.

Changes since v1:

- Dropped MC support from atmel-isi and ov7670: not needed to make this
  work. Only for the ov2640 was it kept since the em28xx driver requires it.
- Use devm_clk_get instead of clk_get.
- The ov7670 lower limit of the clock speed is 10 MHz instead of 12. Adjust
  accordingly.


Hans Verkuil (15):
  ov7670: document device tree bindings
  ov7670: call v4l2_async_register_subdev
  ov7670: fix g/s_parm
  ov7670: get xclk
  ov7670: add devicetree support
  atmel-isi: document device tree bindings
  atmel-isi: remove dependency of the soc-camera framework
  atmel-isi: move out of soc_camera to atmel
  ov2640: update bindings
  ov2640: convert from soc-camera to a standard subdev sensor driver.
  ov2640: use standard clk and enable it.
  ov2640: add MC support
  em28xx: drop last soc_camera link
  sama5d3 dts: enable atmel-isi
  at91-sama5d3_xplained.dts: select ov2640

 .../devicetree/bindings/media/atmel-isi.txt        |   90 +-
 .../devicetree/bindings/media/i2c/ov2640.txt       |   22 +-
 .../devicetree/bindings/media/i2c/ov7670.txt       |   44 +
 MAINTAINERS                                        |    1 +
 arch/arm/boot/dts/at91-sama5d3_xplained.dts        |   60 +-
 arch/arm/boot/dts/sama5d3.dtsi                     |    4 +-
 drivers/media/i2c/Kconfig                          |   11 +
 drivers/media/i2c/Makefile                         |    1 +
 drivers/media/i2c/{soc_camera => }/ov2640.c        |  151 +--
 drivers/media/i2c/ov7670.c                         |   69 +-
 drivers/media/i2c/soc_camera/Kconfig               |    6 -
 drivers/media/i2c/soc_camera/Makefile              |    1 -
 drivers/media/platform/Makefile                    |    1 +
 drivers/media/platform/atmel/Kconfig               |   11 +-
 drivers/media/platform/atmel/Makefile              |    1 +
 drivers/media/platform/atmel/atmel-isi.c           | 1398 ++++++++++++++++++++
 .../platform/{soc_camera => atmel}/atmel-isi.h     |    0
 drivers/media/platform/soc_camera/Kconfig          |   11 -
 drivers/media/platform/soc_camera/Makefile         |    1 -
 drivers/media/platform/soc_camera/atmel-isi.c      | 1167 ----------------
 drivers/media/usb/em28xx/em28xx-camera.c           |    9 -
 21 files changed, 1696 insertions(+), 1363 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7670.txt
 rename drivers/media/i2c/{soc_camera => }/ov2640.c (92%)
 create mode 100644 drivers/media/platform/atmel/atmel-isi.c
 rename drivers/media/platform/{soc_camera => atmel}/atmel-isi.h (100%)
 delete mode 100644 drivers/media/platform/soc_camera/atmel-isi.c

-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 01/15] ov7670: document device tree bindings

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

Add binding documentation and add that file to the MAINTAINERS entry.

Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/media/i2c/ov7670.txt       | 44 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7670.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov7670.txt b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
new file mode 100644
index 000000000000..6d9c90dff7a7
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
@@ -0,0 +1,44 @@
+* Omnivision OV7670 CMOS sensor
+
+The Omnivision OV7670 sensor supports multiple resolutions output, such as
+CIF, SVGA, UXGA. It also can support the YUV422/420, RGB565/555 or raw RGB
+output formats.
+
+Required Properties:
+- compatible: should be "ovti,ov7670"
+- clocks: reference to the xclk input clock.
+- clock-names: should be "xclk".
+
+Optional Properties:
+- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
+  Active is low.
+- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
+  Active is high.
+
+The device node must contain one 'port' child node for its digital output
+video port, in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+	i2c1: i2c@f0018000 {
+		ov7670: camera@21 {
+			compatible = "ovti,ov7670";
+			reg = <0x21>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_pck0_as_isi_mck &pinctrl_sensor_power &pinctrl_sensor_reset>;
+			reset-gpios = <&pioE 11 GPIO_ACTIVE_LOW>;
+			powerdown-gpios = <&pioE 13 GPIO_ACTIVE_HIGH>;
+			clocks = <&pck0>;
+			clock-names = "xclk";
+			assigned-clocks = <&pck0>;
+			assigned-clock-rates = <25000000>;
+
+			port {
+				ov7670_0: endpoint {
+					remote-endpoint = <&isi_0>;
+					bus-width = <8>;
+				};
+			};
+		};
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 83a42ef1d1a7..93500928ca4f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9273,6 +9273,7 @@ L:	linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
 T:	git git://linuxtv.org/media_tree.git
 S:	Maintained
 F:	drivers/media/i2c/ov7670.c
+F:	Documentation/devicetree/bindings/media/i2c/ov7670.txt
 
 ONENAND FLASH DRIVER
 M:	Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 02/15] ov7670: call v4l2_async_register_subdev

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

Add v4l2-async support for this driver.

Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
Acked-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/media/i2c/ov7670.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 56cfb5ca9c95..9af8d3b8f848 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1636,10 +1636,9 @@ static int ov7670_probe(struct i2c_client *client,
 			V4L2_EXPOSURE_AUTO);
 	sd->ctrl_handler = &info->hdl;
 	if (info->hdl.error) {
-		int err = info->hdl.error;
+		ret = info->hdl.error;
 
-		v4l2_ctrl_handler_free(&info->hdl);
-		return err;
+		goto hdl_free;
 	}
 	/*
 	 * We have checked empirically that hw allows to read back the gain
@@ -1651,7 +1650,15 @@ static int ov7670_probe(struct i2c_client *client,
 	v4l2_ctrl_cluster(2, &info->saturation);
 	v4l2_ctrl_handler_setup(&info->hdl);
 
+	ret = v4l2_async_register_subdev(&info->sd);
+	if (ret < 0)
+		goto hdl_free;
+
 	return 0;
+
+hdl_free:
+	v4l2_ctrl_handler_free(&info->hdl);
+	return ret;
 }
 
 
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 03/15] ov7670: fix g/s_parm

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

Drop unnecesary memset. Drop the unnecessary extendedmode check and
set the V4L2_CAP_TIMEPERFRAME capability.

Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
 drivers/media/i2c/ov7670.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 9af8d3b8f848..50e4466a2b37 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1046,7 +1046,6 @@ static int ov7670_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
 	if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
-	memset(cp, 0, sizeof(struct v4l2_captureparm));
 	cp->capability = V4L2_CAP_TIMEPERFRAME;
 	info->devtype->get_framerate(sd, &cp->timeperframe);
 
@@ -1061,9 +1060,8 @@ static int ov7670_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
 
 	if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
-	if (cp->extendedmode != 0)
-		return -EINVAL;
 
+	cp->capability = V4L2_CAP_TIMEPERFRAME;
 	return info->devtype->set_framerate(sd, tpf);
 }
 
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 04/15] ov7670: get xclk

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

Get the clock for this sensor.

Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
 drivers/media/i2c/ov7670.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 50e4466a2b37..da0843617a49 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -10,6 +10,7 @@
  * This file may be distributed under the terms of the GNU General
  * Public License, version 2.
  */
+#include <linux/clk.h>
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/slab.h>
@@ -227,6 +228,7 @@ struct ov7670_info {
 		struct v4l2_ctrl *hue;
 	};
 	struct ov7670_format_struct *fmt;  /* Current format */
+	struct clk *clk;
 	int min_width;			/* Filter out smaller sizes */
 	int min_height;			/* Filter out smaller sizes */
 	int clock_speed;		/* External clock speed (MHz) */
@@ -1587,6 +1589,15 @@ static int ov7670_probe(struct i2c_client *client,
 			info->pclk_hb_disable = true;
 	}
 
+	info->clk = devm_clk_get(&client->dev, "xclk");
+	if (IS_ERR(info->clk))
+		return -EPROBE_DEFER;
+	clk_prepare_enable(info->clk);
+
+	info->clock_speed = clk_get_rate(info->clk) / 1000000;
+	if (info->clock_speed < 10 || info->clock_speed > 48)
+		return -EINVAL;
+
 	/* Make sure it's an ov7670 */
 	ret = ov7670_detect(sd);
 	if (ret) {
@@ -1667,6 +1678,7 @@ static int ov7670_remove(struct i2c_client *client)
 
 	v4l2_device_unregister_subdev(sd);
 	v4l2_ctrl_handler_free(&info->hdl);
+	clk_disable_unprepare(info->clk);
 	return 0;
 }
 
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 05/15] ov7670: add devicetree support

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

Add DT support. Use it to get the reset and pwdn pins (if there are any).
Tested with one sensor requiring reset/pwdn and one sensor that doesn't
have reset/pwdn pins.

Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
 drivers/media/i2c/ov7670.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index da0843617a49..f9d977430dcf 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -17,6 +17,8 @@
 #include <linux/i2c.h>
 #include <linux/delay.h>
 #include <linux/videodev2.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-mediabus.h>
@@ -229,6 +231,8 @@ struct ov7670_info {
 	};
 	struct ov7670_format_struct *fmt;  /* Current format */
 	struct clk *clk;
+	struct gpio_desc *resetb_gpio;
+	struct gpio_desc *pwdn_gpio;
 	int min_width;			/* Filter out smaller sizes */
 	int min_height;			/* Filter out smaller sizes */
 	int clock_speed;		/* External clock speed (MHz) */
@@ -591,8 +595,6 @@ static int ov7670_init(struct v4l2_subdev *sd, u32 val)
 	return ov7670_write_array(sd, ov7670_default_regs);
 }
 
-
-
 static int ov7670_detect(struct v4l2_subdev *sd)
 {
 	unsigned char v;
@@ -1549,6 +1551,27 @@ static const struct ov7670_devtype ov7670_devdata[] = {
 	},
 };
 
+static int ov7670_init_gpio(struct i2c_client *client, struct ov7670_info *info)
+{
+	info->pwdn_gpio = devm_gpiod_get_optional(&client->dev, "powerdown",
+			GPIOD_OUT_LOW);
+	if (IS_ERR(info->pwdn_gpio)) {
+		dev_info(&client->dev, "can't get %s GPIO\n", "powerdown");
+		return PTR_ERR(info->pwdn_gpio);
+	}
+
+	info->resetb_gpio = devm_gpiod_get_optional(&client->dev, "reset",
+			GPIOD_OUT_LOW);
+	if (IS_ERR(info->resetb_gpio)) {
+		dev_info(&client->dev, "can't get %s GPIO\n", "reset");
+		return PTR_ERR(info->resetb_gpio);
+	}
+
+	usleep_range(3000, 5000);
+
+	return 0;
+}
+
 static int ov7670_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
@@ -1594,6 +1617,10 @@ static int ov7670_probe(struct i2c_client *client,
 		return -EPROBE_DEFER;
 	clk_prepare_enable(info->clk);
 
+	ret = ov7670_init_gpio(client, info);
+	if (ret)
+		return ret;
+
 	info->clock_speed = clk_get_rate(info->clk) / 1000000;
 	if (info->clock_speed < 10 || info->clock_speed > 48)
 		return -EINVAL;
@@ -1689,9 +1716,18 @@ static const struct i2c_device_id ov7670_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ov7670_id);
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id ov7670_of_match[] = {
+	{ .compatible = "ovti,ov7670", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ov7670_of_match);
+#endif
+
 static struct i2c_driver ov7670_driver = {
 	.driver = {
 		.name	= "ov7670",
+		.of_match_table = of_match_ptr(ov7670_of_match),
 	},
 	.probe		= ov7670_probe,
 	.remove		= ov7670_remove,
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 06/15] atmel-isi: document device tree bindings

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

Document the device tree bindings for this hardware.

Mostly copied from the atmel-isc bindings.

Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/media/atmel-isi.txt        | 90 +++++++++++++---------
 1 file changed, 55 insertions(+), 35 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt b/Documentation/devicetree/bindings/media/atmel-isi.txt
index 251f008f220c..d6e93e8216af 100644
--- a/Documentation/devicetree/bindings/media/atmel-isi.txt
+++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
@@ -1,51 +1,71 @@
-Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
-----------------------------------------------
+Atmel Image Sensor Interface (ISI)
+----------------------------------
 
-Required properties:
-- compatible: must be "atmel,at91sam9g45-isi"
-- reg: physical base address and length of the registers set for the device;
-- interrupts: should contain IRQ line for the ISI;
-- clocks: list of clock specifiers, corresponding to entries in
-          the clock-names property;
-- clock-names: must contain "isi_clk", which is the isi peripherial clock.
+Required properties for ISI:
+- compatible: must be "atmel,at91sam9g45-isi".
+- reg: physical base address and length of the registers set for the device.
+- interrupts: should contain IRQ line for the ISI.
+- clocks: list of clock specifiers, corresponding to entries in the clock-names
+	property; please refer to clock-bindings.txt.
+- clock-names: required elements: "isi_clk".
+- pinctrl-names, pinctrl-0: please refer to pinctrl-bindings.txt.
 
 ISI supports a single port node with parallel bus. It should contain one
 'port' child node with child 'endpoint' node. Please refer to the bindings
 defined in Documentation/devicetree/bindings/media/video-interfaces.txt.
 
-Example:
-	isi: isi@f0034000 {
-		compatible = "atmel,at91sam9g45-isi";
-		reg = <0xf0034000 0x4000>;
-		interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
-
-		clocks = <&isi_clk>;
-		clock-names = "isi_clk";
+Endpoint node properties
+------------------------
 
-		pinctrl-names = "default";
-		pinctrl-0 = <&pinctrl_isi>;
+- bus-width: <8> or <10> (mandatory)
+- hsync-active
+- vsync-active
+- pclk-sample
+- remote-endpoint: A phandle to the bus receiver's endpoint node.
 
-		port {
-			#address-cells = <1>;
-			#size-cells = <0>;
+Example:
 
-			isi_0: endpoint {
-				remote-endpoint = <&ov2640_0>;
-				bus-width = <8>;
-			};
+isi: isi@f0034000 {
+	compatible = "atmel,at91sam9g45-isi";
+	reg = <0xf0034000 0x4000>;
+	interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_isi_data_0_7>;
+	clocks = <&isi_clk>;
+	clock-names = "isi_clk";
+	status = "ok";
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		isi_0: endpoint {
+			remote-endpoint = <&ov2640_0>;
+			bus-width = <8>;
+			vsync-active = <1>;
+			hsync-active = <1>;
 		};
 	};
+};
+
+i2c1: i2c@f0018000 {
+	status = "okay";
 
-	i2c1: i2c@f0018000 {
-		ov2640: camera@0x30 {
-			compatible = "ovti,ov2640";
-			reg = <0x30>;
+	ov2640: camera@30 {
+		compatible = "ovti,ov2640";
+		reg = <0x30>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pck0_as_isi_mck &pinctrl_sensor_power &pinctrl_sensor_reset>;
+		resetb-gpios = <&pioE 11 GPIO_ACTIVE_LOW>;
+		pwdn-gpios = <&pioE 13 GPIO_ACTIVE_HIGH>;
+		clocks = <&pck0>;
+		clock-names = "xvclk";
+		assigned-clocks = <&pck0>;
+		assigned-clock-rates = <25000000>;
 
-			port {
-				ov2640_0: endpoint {
-					remote-endpoint = <&isi_0>;
-					bus-width = <8>;
-				};
+		port {
+			ov2640_0: endpoint {
+				remote-endpoint = <&isi_0>;
+				bus-width = <8>;
 			};
 		};
 	};
+};
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 07/15] atmel-isi: remove dependency of the soc-camera framework

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

This patch converts the atmel-isi driver from a soc-camera driver to a driver
that is stand-alone.

Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
 drivers/media/i2c/soc_camera/ov2640.c         |   23 +-
 drivers/media/platform/soc_camera/Kconfig     |    3 +-
 drivers/media/platform/soc_camera/atmel-isi.c | 1223 +++++++++++++++----------
 3 files changed, 735 insertions(+), 514 deletions(-)

diff --git a/drivers/media/i2c/soc_camera/ov2640.c b/drivers/media/i2c/soc_camera/ov2640.c
index 56de18263359..b9a0069f5b33 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -794,10 +794,11 @@ static int ov2640_set_params(struct i2c_client *client, u32 *width, u32 *height,
 		dev_dbg(&client->dev, "%s: Selected cfmt YUYV (YUV422)", __func__);
 		selected_cfmt_regs = ov2640_yuyv_regs;
 		break;
-	default:
 	case MEDIA_BUS_FMT_UYVY8_2X8:
+	default:
 		dev_dbg(&client->dev, "%s: Selected cfmt UYVY", __func__);
 		selected_cfmt_regs = ov2640_uyvy_regs;
+		break;
 	}
 
 	/* reset hardware */
@@ -865,17 +866,7 @@ static int ov2640_get_fmt(struct v4l2_subdev *sd,
 	mf->width	= priv->win->width;
 	mf->height	= priv->win->height;
 	mf->code	= priv->cfmt_code;
-
-	switch (mf->code) {
-	case MEDIA_BUS_FMT_RGB565_2X8_BE:
-	case MEDIA_BUS_FMT_RGB565_2X8_LE:
-		mf->colorspace = V4L2_COLORSPACE_SRGB;
-		break;
-	default:
-	case MEDIA_BUS_FMT_YUYV8_2X8:
-	case MEDIA_BUS_FMT_UYVY8_2X8:
-		mf->colorspace = V4L2_COLORSPACE_JPEG;
-	}
+	mf->colorspace	= V4L2_COLORSPACE_SRGB;
 	mf->field	= V4L2_FIELD_NONE;
 
 	return 0;
@@ -897,17 +888,17 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
 	ov2640_select_win(&mf->width, &mf->height);
 
 	mf->field	= V4L2_FIELD_NONE;
+	mf->colorspace	= V4L2_COLORSPACE_SRGB;
 
 	switch (mf->code) {
 	case MEDIA_BUS_FMT_RGB565_2X8_BE:
 	case MEDIA_BUS_FMT_RGB565_2X8_LE:
-		mf->colorspace = V4L2_COLORSPACE_SRGB;
+	case MEDIA_BUS_FMT_YUYV8_2X8:
+	case MEDIA_BUS_FMT_UYVY8_2X8:
 		break;
 	default:
 		mf->code = MEDIA_BUS_FMT_UYVY8_2X8;
-	case MEDIA_BUS_FMT_YUYV8_2X8:
-	case MEDIA_BUS_FMT_UYVY8_2X8:
-		mf->colorspace = V4L2_COLORSPACE_JPEG;
+		break;
 	}
 
 	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
diff --git a/drivers/media/platform/soc_camera/Kconfig b/drivers/media/platform/soc_camera/Kconfig
index 86d74788544f..a37ec91b026e 100644
--- a/drivers/media/platform/soc_camera/Kconfig
+++ b/drivers/media/platform/soc_camera/Kconfig
@@ -29,9 +29,8 @@ config VIDEO_SH_MOBILE_CEU
 
 config VIDEO_ATMEL_ISI
 	tristate "ATMEL Image Sensor Interface (ISI) support"
-	depends on VIDEO_DEV && SOC_CAMERA
+	depends on VIDEO_V4L2 && OF && HAS_DMA
 	depends on ARCH_AT91 || COMPILE_TEST
-	depends on HAS_DMA
 	select VIDEOBUF2_DMA_CONTIG
 	---help---
 	  This module makes the ATMEL Image Sensor Interface available
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index 46de657c3e6d..a837b94281ef 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -22,18 +22,22 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
-
-#include <media/soc_camera.h>
-#include <media/drv-intf/soc_mediabus.h>
+#include <linux/of.h>
+
+#include <linux/videodev2.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-dev.h>
+#include <media/v4l2-ioctl.h>
+#include <media/v4l2-event.h>
 #include <media/v4l2-of.h>
 #include <media/videobuf2-dma-contig.h>
+#include <media/v4l2-image-sizes.h>
 
 #include "atmel-isi.h"
 
-#define MAX_BUFFER_NUM			32
 #define MAX_SUPPORT_WIDTH		2048
 #define MAX_SUPPORT_HEIGHT		2048
-#define VID_LIMIT_BYTES			(16 * 1024 * 1024)
 #define MIN_FRAME_RATE			15
 #define FRAME_INTERVAL_MILLI_SEC	(1000 / MIN_FRAME_RATE)
 
@@ -65,9 +69,37 @@ struct frame_buffer {
 	struct list_head list;
 };
 
+struct isi_graph_entity {
+	struct device_node *node;
+
+	struct v4l2_async_subdev asd;
+	struct v4l2_subdev *subdev;
+};
+
+/*
+ * struct isi_format - ISI media bus format information
+ * @fourcc:		Fourcc code for this format
+ * @mbus_code:		V4L2 media bus format code.
+ * @bpp:		Bytes per pixel (when stored in memory)
+ * @swap:		Byte swap configuration value
+ * @support:		Indicates format supported by subdev
+ * @skip:		Skip duplicate format supported by subdev
+ */
+struct isi_format {
+	u32	fourcc;
+	u32	mbus_code;
+	u8	bpp;
+	u32	swap;
+
+	bool	support;
+	bool	skip;
+};
+
+
 struct atmel_isi {
 	/* Protects the access of variables shared with the ISR */
-	spinlock_t			lock;
+	spinlock_t			irqlock;
+	struct device			*dev;
 	void __iomem			*regs;
 
 	int				sequence;
@@ -76,7 +108,7 @@ struct atmel_isi {
 	struct fbd			*p_fb_descriptors;
 	dma_addr_t			fb_descriptors_phys;
 	struct				list_head dma_desc_head;
-	struct isi_dma_desc		dma_desc[MAX_BUFFER_NUM];
+	struct isi_dma_desc		dma_desc[VIDEO_MAX_FRAME];
 	bool				enable_preview_path;
 
 	struct completion		complete;
@@ -90,9 +122,22 @@ struct atmel_isi {
 	struct list_head		video_buffer_list;
 	struct frame_buffer		*active;
 
-	struct soc_camera_host		soc_host;
+	struct v4l2_device		v4l2_dev;
+	struct video_device		*vdev;
+	struct v4l2_async_notifier	notifier;
+	struct isi_graph_entity		entity;
+	struct v4l2_format		fmt;
+
+	struct isi_format		**user_formats;
+	unsigned int			num_user_formats;
+	const struct isi_format		*current_fmt;
+
+	struct mutex			lock;
+	struct vb2_queue		queue;
 };
 
+#define notifier_to_isi(n) container_of(n, struct atmel_isi, notifier)
+
 static void isi_writel(struct atmel_isi *isi, u32 reg, u32 val)
 {
 	writel(val, isi->regs + reg);
@@ -102,107 +147,46 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
 	return readl(isi->regs + reg);
 }
 
-static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
-		const struct soc_camera_format_xlate *xlate)
-{
-	if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_YUYV) {
-		/* all convert to YUYV */
-		switch (xlate->code) {
-		case MEDIA_BUS_FMT_VYUY8_2X8:
-			return ISI_CFG2_YCC_SWAP_MODE_3;
-		case MEDIA_BUS_FMT_UYVY8_2X8:
-			return ISI_CFG2_YCC_SWAP_MODE_2;
-		case MEDIA_BUS_FMT_YVYU8_2X8:
-			return ISI_CFG2_YCC_SWAP_MODE_1;
-		}
-	} else if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_RGB565) {
-		/*
-		 * Preview path is enabled, it will convert UYVY to RGB format.
-		 * But if sensor output format is not UYVY, we need to set
-		 * YCC_SWAP_MODE to convert it as UYVY.
-		 */
-		switch (xlate->code) {
-		case MEDIA_BUS_FMT_VYUY8_2X8:
-			return ISI_CFG2_YCC_SWAP_MODE_1;
-		case MEDIA_BUS_FMT_YUYV8_2X8:
-			return ISI_CFG2_YCC_SWAP_MODE_2;
-		case MEDIA_BUS_FMT_YVYU8_2X8:
-			return ISI_CFG2_YCC_SWAP_MODE_3;
-		}
-	}
-
-	/*
-	 * By default, no swap for the codec path of Atmel ISI. So codec
-	 * output is same as sensor's output.
-	 * For instance, if sensor's output is YUYV, then codec outputs YUYV.
-	 * And if sensor's output is UYVY, then codec outputs UYVY.
-	 */
-	return ISI_CFG2_YCC_SWAP_DEFAULT;
-}
+static struct isi_format isi_formats[] = {
+	{ V4L2_PIX_FMT_YUYV, MEDIA_BUS_FMT_YUYV8_2X8,
+	  2, ISI_CFG2_YCC_SWAP_DEFAULT, false },
+	{ V4L2_PIX_FMT_YUYV, MEDIA_BUS_FMT_YVYU8_2X8,
+	  2, ISI_CFG2_YCC_SWAP_MODE_1, false },
+	{ V4L2_PIX_FMT_YUYV, MEDIA_BUS_FMT_UYVY8_2X8,
+	  2, ISI_CFG2_YCC_SWAP_MODE_2, false },
+	{ V4L2_PIX_FMT_YUYV, MEDIA_BUS_FMT_VYUY8_2X8,
+	  2, ISI_CFG2_YCC_SWAP_MODE_3, false },
+	{ V4L2_PIX_FMT_RGB565, MEDIA_BUS_FMT_YUYV8_2X8,
+	  2, ISI_CFG2_YCC_SWAP_MODE_2, false },
+};
 
-static void configure_geometry(struct atmel_isi *isi, u32 width,
-		u32 height, const struct soc_camera_format_xlate *xlate)
+static void configure_geometry(struct atmel_isi *isi)
 {
-	u32 cfg2, psize;
-	u32 fourcc = xlate->host_fmt->fourcc;
+	u32 cfg2 = 0, psize;
+	u32 fourcc = isi->current_fmt->fourcc;
 
 	isi->enable_preview_path = fourcc == V4L2_PIX_FMT_RGB565 ||
 				   fourcc == V4L2_PIX_FMT_RGB32;
 
 	/* According to sensor's output format to set cfg2 */
-	switch (xlate->code) {
-	default:
-	/* Grey */
-	case MEDIA_BUS_FMT_Y8_1X8:
-		cfg2 = ISI_CFG2_GRAYSCALE | ISI_CFG2_COL_SPACE_YCbCr;
-		break;
-	/* YUV */
-	case MEDIA_BUS_FMT_VYUY8_2X8:
-	case MEDIA_BUS_FMT_UYVY8_2X8:
-	case MEDIA_BUS_FMT_YVYU8_2X8:
-	case MEDIA_BUS_FMT_YUYV8_2X8:
-		cfg2 = ISI_CFG2_COL_SPACE_YCbCr |
-				setup_cfg2_yuv_swap(isi, xlate);
-		break;
-	/* RGB, TODO */
-	}
+	cfg2 = isi->current_fmt->swap;
 
 	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
 	/* Set width */
-	cfg2 |= ((width - 1) << ISI_CFG2_IM_HSIZE_OFFSET) &
+	cfg2 |= ((isi->fmt.fmt.pix.width - 1) << ISI_CFG2_IM_HSIZE_OFFSET) &
 			ISI_CFG2_IM_HSIZE_MASK;
 	/* Set height */
-	cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
+	cfg2 |= ((isi->fmt.fmt.pix.height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
 			& ISI_CFG2_IM_VSIZE_MASK;
 	isi_writel(isi, ISI_CFG2, cfg2);
 
 	/* No down sampling, preview size equal to sensor output size */
-	psize = ((width - 1) << ISI_PSIZE_PREV_HSIZE_OFFSET) &
+	psize = ((isi->fmt.fmt.pix.width - 1) << ISI_PSIZE_PREV_HSIZE_OFFSET) &
 		ISI_PSIZE_PREV_HSIZE_MASK;
-	psize |= ((height - 1) << ISI_PSIZE_PREV_VSIZE_OFFSET) &
+	psize |= ((isi->fmt.fmt.pix.height - 1) << ISI_PSIZE_PREV_VSIZE_OFFSET) &
 		ISI_PSIZE_PREV_VSIZE_MASK;
 	isi_writel(isi, ISI_PSIZE, psize);
 	isi_writel(isi, ISI_PDECF, ISI_PDECF_NO_SAMPLING);
-
-	return;
-}
-
-static bool is_supported(struct soc_camera_device *icd,
-		const u32 pixformat)
-{
-	switch (pixformat) {
-	/* YUV, including grey */
-	case V4L2_PIX_FMT_GREY:
-	case V4L2_PIX_FMT_YUYV:
-	case V4L2_PIX_FMT_UYVY:
-	case V4L2_PIX_FMT_YVYU:
-	case V4L2_PIX_FMT_VYUY:
-	/* RGB */
-	case V4L2_PIX_FMT_RGB565:
-		return true;
-	default:
-		return false;
-	}
 }
 
 static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
@@ -214,6 +198,7 @@ static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
 		list_del_init(&buf->list);
 		vbuf->vb2_buf.timestamp = ktime_get_ns();
 		vbuf->sequence = isi->sequence++;
+		vbuf->field = V4L2_FIELD_NONE;
 		vb2_buffer_done(&vbuf->vb2_buf, VB2_BUF_STATE_DONE);
 	}
 
@@ -247,7 +232,7 @@ static irqreturn_t isi_interrupt(int irq, void *dev_id)
 	u32 status, mask, pending;
 	irqreturn_t ret = IRQ_NONE;
 
-	spin_lock(&isi->lock);
+	spin_lock(&isi->irqlock);
 
 	status = isi_readl(isi, ISI_STATUS);
 	mask = isi_readl(isi, ISI_INTMASK);
@@ -267,7 +252,7 @@ static irqreturn_t isi_interrupt(int irq, void *dev_id)
 			ret = atmel_isi_handle_streaming(isi);
 	}
 
-	spin_unlock(&isi->lock);
+	spin_unlock(&isi->irqlock);
 	return ret;
 }
 
@@ -305,26 +290,21 @@ static int queue_setup(struct vb2_queue *vq,
 				unsigned int *nbuffers, unsigned int *nplanes,
 				unsigned int sizes[], struct device *alloc_devs[])
 {
-	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct atmel_isi *isi = ici->priv;
+	struct atmel_isi *isi = vb2_get_drv_priv(vq);
 	unsigned long size;
 
-	size = icd->sizeimage;
-
-	if (!*nbuffers || *nbuffers > MAX_BUFFER_NUM)
-		*nbuffers = MAX_BUFFER_NUM;
+	size = isi->fmt.fmt.pix.sizeimage;
 
-	if (size * *nbuffers > VID_LIMIT_BYTES)
-		*nbuffers = VID_LIMIT_BYTES / size;
+	/* Make sure the image size is large enough. */
+	if (*nplanes)
+		return sizes[0] < size ? -EINVAL : 0;
 
 	*nplanes = 1;
 	sizes[0] = size;
 
-	isi->sequence = 0;
 	isi->active = NULL;
 
-	dev_dbg(icd->parent, "%s, count=%d, size=%ld\n", __func__,
+	dev_dbg(isi->dev, "%s, count=%d, size=%ld\n", __func__,
 		*nbuffers, size);
 
 	return 0;
@@ -344,17 +324,15 @@ static int buffer_init(struct vb2_buffer *vb)
 static int buffer_prepare(struct vb2_buffer *vb)
 {
 	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
-	struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
 	struct frame_buffer *buf = container_of(vbuf, struct frame_buffer, vb);
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct atmel_isi *isi = ici->priv;
+	struct atmel_isi *isi = vb2_get_drv_priv(vb->vb2_queue);
 	unsigned long size;
 	struct isi_dma_desc *desc;
 
-	size = icd->sizeimage;
+	size = isi->fmt.fmt.pix.sizeimage;
 
 	if (vb2_plane_size(vb, 0) < size) {
-		dev_err(icd->parent, "%s data will not fit into plane (%lu < %lu)\n",
+		dev_err(isi->dev, "%s data will not fit into plane (%lu < %lu)\n",
 				__func__, vb2_plane_size(vb, 0), size);
 		return -EINVAL;
 	}
@@ -363,7 +341,7 @@ static int buffer_prepare(struct vb2_buffer *vb)
 
 	if (!buf->p_dma_desc) {
 		if (list_empty(&isi->dma_desc_head)) {
-			dev_err(icd->parent, "Not enough dma descriptors.\n");
+			dev_err(isi->dev, "Not enough dma descriptors.\n");
 			return -EINVAL;
 		} else {
 			/* Get an available descriptor */
@@ -387,9 +365,7 @@ static int buffer_prepare(struct vb2_buffer *vb)
 static void buffer_cleanup(struct vb2_buffer *vb)
 {
 	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
-	struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct atmel_isi *isi = ici->priv;
+	struct atmel_isi *isi = vb2_get_drv_priv(vb->vb2_queue);
 	struct frame_buffer *buf = container_of(vbuf, struct frame_buffer, vb);
 
 	/* This descriptor is available now and we add to head list */
@@ -409,7 +385,7 @@ static void start_dma(struct atmel_isi *isi, struct frame_buffer *buffer)
 	/* Check if already in a frame */
 	if (!isi->enable_preview_path) {
 		if (isi_readl(isi, ISI_STATUS) & ISI_CTRL_CDC) {
-			dev_err(isi->soc_host.icd->parent, "Already in frame handling.\n");
+			dev_err(isi->dev, "Already in frame handling.\n");
 			return;
 		}
 
@@ -443,13 +419,11 @@ static void start_dma(struct atmel_isi *isi, struct frame_buffer *buffer)
 static void buffer_queue(struct vb2_buffer *vb)
 {
 	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
-	struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct atmel_isi *isi = ici->priv;
+	struct atmel_isi *isi = vb2_get_drv_priv(vb->vb2_queue);
 	struct frame_buffer *buf = container_of(vbuf, struct frame_buffer, vb);
 	unsigned long flags = 0;
 
-	spin_lock_irqsave(&isi->lock, flags);
+	spin_lock_irqsave(&isi->irqlock, flags);
 	list_add_tail(&buf->list, &isi->video_buffer_list);
 
 	if (isi->active == NULL) {
@@ -457,60 +431,83 @@ static void buffer_queue(struct vb2_buffer *vb)
 		if (vb2_is_streaming(vb->vb2_queue))
 			start_dma(isi, buf);
 	}
-	spin_unlock_irqrestore(&isi->lock, flags);
+	spin_unlock_irqrestore(&isi->irqlock, flags);
 }
 
 static int start_streaming(struct vb2_queue *vq, unsigned int count)
 {
-	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct atmel_isi *isi = ici->priv;
+	struct atmel_isi *isi = vb2_get_drv_priv(vq);
+	struct frame_buffer *buf, *node;
 	int ret;
 
-	pm_runtime_get_sync(ici->v4l2_dev.dev);
+	/* Enable stream on the sub device */
+	ret = v4l2_subdev_call(isi->entity.subdev, video, s_stream, 1);
+	if (ret && ret != -ENOIOCTLCMD) {
+		v4l2_err(&isi->v4l2_dev, "stream on failed in subdev\n");
+		goto err_start_stream;
+	}
+
+	pm_runtime_get_sync(isi->dev);
 
 	/* Reset ISI */
 	ret = atmel_isi_wait_status(isi, WAIT_ISI_RESET);
 	if (ret < 0) {
-		dev_err(icd->parent, "Reset ISI timed out\n");
-		pm_runtime_put(ici->v4l2_dev.dev);
-		return ret;
+		dev_err(isi->dev, "Reset ISI timed out\n");
+		goto err_reset;
 	}
 	/* Disable all interrupts */
 	isi_writel(isi, ISI_INTDIS, (u32)~0UL);
 
-	configure_geometry(isi, icd->user_width, icd->user_height,
-				icd->current_fmt);
+	isi->sequence = 0;
+	configure_geometry(isi);
 
-	spin_lock_irq(&isi->lock);
+	spin_lock_irq(&isi->irqlock);
 	/* Clear any pending interrupt */
 	isi_readl(isi, ISI_STATUS);
 
-	if (count)
-		start_dma(isi, isi->active);
-	spin_unlock_irq(&isi->lock);
+	start_dma(isi, isi->active);
+	spin_unlock_irq(&isi->irqlock);
 
 	return 0;
+
+err_reset:
+	pm_runtime_put(isi->dev);
+	v4l2_subdev_call(isi->entity.subdev, video, s_stream, 0);
+
+err_start_stream:
+	spin_lock_irq(&isi->irqlock);
+	isi->active = NULL;
+	/* Release all active buffers */
+	list_for_each_entry_safe(buf, node, &isi->video_buffer_list, list) {
+		list_del_init(&buf->list);
+		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);
+	}
+	spin_unlock_irq(&isi->irqlock);
+
+	return ret;
 }
 
 /* abort streaming and wait for last buffer */
 static void stop_streaming(struct vb2_queue *vq)
 {
-	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct atmel_isi *isi = ici->priv;
+	struct atmel_isi *isi = vb2_get_drv_priv(vq);
 	struct frame_buffer *buf, *node;
 	int ret = 0;
 	unsigned long timeout;
 
-	spin_lock_irq(&isi->lock);
+	/* Disable stream on the sub device */
+	ret = v4l2_subdev_call(isi->entity.subdev, video, s_stream, 0);
+	if (ret && ret != -ENOIOCTLCMD)
+		v4l2_err(&isi->v4l2_dev, "stream off failed in subdev\n");
+
+	spin_lock_irq(&isi->irqlock);
 	isi->active = NULL;
 	/* Release all active buffers */
 	list_for_each_entry_safe(buf, node, &isi->video_buffer_list, list) {
 		list_del_init(&buf->list);
 		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
 	}
-	spin_unlock_irq(&isi->lock);
+	spin_unlock_irq(&isi->irqlock);
 
 	if (!isi->enable_preview_path) {
 		timeout = jiffies + FRAME_INTERVAL_MILLI_SEC * HZ;
@@ -520,7 +517,7 @@ static void stop_streaming(struct vb2_queue *vq)
 			msleep(1);
 
 		if (time_after(jiffies, timeout))
-			dev_err(icd->parent,
+			dev_err(isi->dev,
 				"Timeout waiting for finishing codec request\n");
 	}
 
@@ -531,9 +528,9 @@ static void stop_streaming(struct vb2_queue *vq)
 	/* Disable ISI and wait for it is done */
 	ret = atmel_isi_wait_status(isi, WAIT_ISI_DISABLE);
 	if (ret < 0)
-		dev_err(icd->parent, "Disable ISI timed out\n");
+		dev_err(isi->dev, "Disable ISI timed out\n");
 
-	pm_runtime_put(ici->v4l2_dev.dev);
+	pm_runtime_put(isi->dev);
 }
 
 static const struct vb2_ops isi_video_qops = {
@@ -548,380 +545,257 @@ static const struct vb2_ops isi_video_qops = {
 	.wait_finish		= vb2_ops_wait_finish,
 };
 
-/* ------------------------------------------------------------------
-	SOC camera operations for the device
-   ------------------------------------------------------------------*/
-static int isi_camera_init_videobuf(struct vb2_queue *q,
-				     struct soc_camera_device *icd)
+static int isi_g_fmt_vid_cap(struct file *file, void *priv,
+			      struct v4l2_format *fmt)
 {
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+	struct atmel_isi *isi = video_drvdata(file);
 
-	q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-	q->io_modes = VB2_MMAP;
-	q->drv_priv = icd;
-	q->buf_struct_size = sizeof(struct frame_buffer);
-	q->ops = &isi_video_qops;
-	q->mem_ops = &vb2_dma_contig_memops;
-	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
-	q->lock = &ici->host_lock;
-	q->dev = ici->v4l2_dev.dev;
+	*fmt = isi->fmt;
+
+	return 0;
+}
+
+static struct isi_format *find_format_by_fourcc(struct atmel_isi *isi,
+						 unsigned int fourcc)
+{
+	unsigned int num_formats = isi->num_user_formats;
+	struct isi_format *fmt;
+	unsigned int i;
+
+	for (i = 0; i < num_formats; i++) {
+		fmt = isi->user_formats[i];
+		if (fmt->fourcc == fourcc)
+			return fmt;
+	}
 
-	return vb2_queue_init(q);
+	return NULL;
 }
 
-static int isi_camera_set_fmt(struct soc_camera_device *icd,
-			      struct v4l2_format *f)
+static int isi_try_fmt(struct atmel_isi *isi, struct v4l2_format *f,
+			struct isi_format **current_fmt)
 {
-	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
-	const struct soc_camera_format_xlate *xlate;
-	struct v4l2_pix_format *pix = &f->fmt.pix;
+	struct isi_format *isi_fmt;
+	struct v4l2_pix_format *pixfmt = &f->fmt.pix;
+	struct v4l2_subdev_pad_config pad_cfg;
 	struct v4l2_subdev_format format = {
-		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+		.which = V4L2_SUBDEV_FORMAT_TRY,
 	};
-	struct v4l2_mbus_framefmt *mf = &format.format;
 	int ret;
 
-	/* check with atmel-isi support format, if not support use YUYV */
-	if (!is_supported(icd, pix->pixelformat))
-		pix->pixelformat = V4L2_PIX_FMT_YUYV;
-
-	xlate = soc_camera_xlate_by_fourcc(icd, pix->pixelformat);
-	if (!xlate) {
-		dev_warn(icd->parent, "Format %x not found\n",
-			 pix->pixelformat);
+	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
-	}
 
-	dev_dbg(icd->parent, "Plan to set format %dx%d\n",
-			pix->width, pix->height);
+	isi_fmt = find_format_by_fourcc(isi, pixfmt->pixelformat);
+	if (!isi_fmt) {
+		isi_fmt = isi->user_formats[isi->num_user_formats - 1];
+		pixfmt->pixelformat = isi_fmt->fourcc;
+	}
 
-	mf->width	= pix->width;
-	mf->height	= pix->height;
-	mf->field	= pix->field;
-	mf->colorspace	= pix->colorspace;
-	mf->code	= xlate->code;
+	/* Limit to Atmel ISC hardware capabilities */
+	if (pixfmt->width > MAX_SUPPORT_WIDTH)
+		pixfmt->width = MAX_SUPPORT_WIDTH;
+	if (pixfmt->height > MAX_SUPPORT_HEIGHT)
+		pixfmt->height = MAX_SUPPORT_HEIGHT;
 
-	ret = v4l2_subdev_call(sd, pad, set_fmt, NULL, &format);
+	v4l2_fill_mbus_format(&format.format, pixfmt, isi_fmt->mbus_code);
+	ret = v4l2_subdev_call(isi->entity.subdev, pad, set_fmt,
+			       &pad_cfg, &format);
 	if (ret < 0)
 		return ret;
 
-	if (mf->code != xlate->code)
-		return -EINVAL;
+	v4l2_fill_pix_format(pixfmt, &format.format);
 
-	pix->width		= mf->width;
-	pix->height		= mf->height;
-	pix->field		= mf->field;
-	pix->colorspace		= mf->colorspace;
-	icd->current_fmt	= xlate;
+	pixfmt->field = V4L2_FIELD_NONE;
+	pixfmt->bytesperline = pixfmt->width * isi_fmt->bpp;
+	pixfmt->sizeimage = pixfmt->bytesperline * pixfmt->height;
 
-	dev_dbg(icd->parent, "Finally set format %dx%d\n",
-		pix->width, pix->height);
+	if (current_fmt)
+		*current_fmt = isi_fmt;
 
-	return ret;
+	return 0;
 }
 
-static int isi_camera_try_fmt(struct soc_camera_device *icd,
-			      struct v4l2_format *f)
+static int isi_set_fmt(struct atmel_isi *isi, struct v4l2_format *f)
 {
-	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
-	const struct soc_camera_format_xlate *xlate;
-	struct v4l2_pix_format *pix = &f->fmt.pix;
-	struct v4l2_subdev_pad_config pad_cfg;
 	struct v4l2_subdev_format format = {
-		.which = V4L2_SUBDEV_FORMAT_TRY,
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
 	};
-	struct v4l2_mbus_framefmt *mf = &format.format;
-	u32 pixfmt = pix->pixelformat;
+	struct isi_format *current_fmt;
 	int ret;
 
-	/* check with atmel-isi support format, if not support use YUYV */
-	if (!is_supported(icd, pix->pixelformat))
-		pix->pixelformat = V4L2_PIX_FMT_YUYV;
-
-	xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
-	if (pixfmt && !xlate) {
-		dev_warn(icd->parent, "Format %x not found\n", pixfmt);
-		return -EINVAL;
-	}
-
-	/* limit to Atmel ISI hardware capabilities */
-	if (pix->height > MAX_SUPPORT_HEIGHT)
-		pix->height = MAX_SUPPORT_HEIGHT;
-	if (pix->width > MAX_SUPPORT_WIDTH)
-		pix->width = MAX_SUPPORT_WIDTH;
-
-	/* limit to sensor capabilities */
-	mf->width	= pix->width;
-	mf->height	= pix->height;
-	mf->field	= pix->field;
-	mf->colorspace	= pix->colorspace;
-	mf->code	= xlate->code;
+	ret = isi_try_fmt(isi, f, &current_fmt);
+	if (ret)
+		return ret;
 
-	ret = v4l2_subdev_call(sd, pad, set_fmt, &pad_cfg, &format);
+	v4l2_fill_mbus_format(&format.format, &f->fmt.pix,
+			      current_fmt->mbus_code);
+	ret = v4l2_subdev_call(isi->entity.subdev, pad,
+			       set_fmt, NULL, &format);
 	if (ret < 0)
 		return ret;
 
-	pix->width	= mf->width;
-	pix->height	= mf->height;
-	pix->colorspace	= mf->colorspace;
+	isi->fmt = *f;
+	isi->current_fmt = current_fmt;
 
-	switch (mf->field) {
-	case V4L2_FIELD_ANY:
-		pix->field = V4L2_FIELD_NONE;
-		break;
-	case V4L2_FIELD_NONE:
-		break;
-	default:
-		dev_err(icd->parent, "Field type %d unsupported.\n",
-			mf->field);
-		ret = -EINVAL;
-	}
-
-	return ret;
+	return 0;
 }
 
-static const struct soc_mbus_pixelfmt isi_camera_formats[] = {
-	{
-		.fourcc			= V4L2_PIX_FMT_YUYV,
-		.name			= "Packed YUV422 16 bit",
-		.bits_per_sample	= 8,
-		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
-		.order			= SOC_MBUS_ORDER_LE,
-		.layout			= SOC_MBUS_LAYOUT_PACKED,
-	},
-	{
-		.fourcc			= V4L2_PIX_FMT_RGB565,
-		.name			= "RGB565",
-		.bits_per_sample	= 8,
-		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
-		.order			= SOC_MBUS_ORDER_LE,
-		.layout			= SOC_MBUS_LAYOUT_PACKED,
-	},
-};
-
-/* This will be corrected as we get more formats */
-static bool isi_camera_packing_supported(const struct soc_mbus_pixelfmt *fmt)
+static int isi_s_fmt_vid_cap(struct file *file, void *priv,
+			      struct v4l2_format *f)
 {
-	return	fmt->packing == SOC_MBUS_PACKING_NONE ||
-		(fmt->bits_per_sample == 8 &&
-		 fmt->packing == SOC_MBUS_PACKING_2X8_PADHI) ||
-		(fmt->bits_per_sample > 8 &&
-		 fmt->packing == SOC_MBUS_PACKING_EXTEND16);
+	struct atmel_isi *isi = video_drvdata(file);
+
+	if (vb2_is_streaming(&isi->queue))
+		return -EBUSY;
+
+	return isi_set_fmt(isi, f);
 }
 
-#define ISI_BUS_PARAM (V4L2_MBUS_MASTER |	\
-		V4L2_MBUS_HSYNC_ACTIVE_HIGH |	\
-		V4L2_MBUS_HSYNC_ACTIVE_LOW |	\
-		V4L2_MBUS_VSYNC_ACTIVE_HIGH |	\
-		V4L2_MBUS_VSYNC_ACTIVE_LOW |	\
-		V4L2_MBUS_PCLK_SAMPLE_RISING |	\
-		V4L2_MBUS_PCLK_SAMPLE_FALLING |	\
-		V4L2_MBUS_DATA_ACTIVE_HIGH)
-
-static int isi_camera_try_bus_param(struct soc_camera_device *icd,
-				    unsigned char buswidth)
+static int isi_try_fmt_vid_cap(struct file *file, void *priv,
+				struct v4l2_format *f)
 {
-	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct atmel_isi *isi = ici->priv;
-	struct v4l2_mbus_config cfg = {.type = V4L2_MBUS_PARALLEL,};
-	unsigned long common_flags;
-	int ret;
-
-	ret = v4l2_subdev_call(sd, video, g_mbus_config, &cfg);
-	if (!ret) {
-		common_flags = soc_mbus_config_compatible(&cfg,
-							  ISI_BUS_PARAM);
-		if (!common_flags) {
-			dev_warn(icd->parent,
-				 "Flags incompatible: camera 0x%x, host 0x%x\n",
-				 cfg.flags, ISI_BUS_PARAM);
-			return -EINVAL;
-		}
-	} else if (ret != -ENOIOCTLCMD) {
-		return ret;
-	}
+	struct atmel_isi *isi = video_drvdata(file);
 
-	if ((1 << (buswidth - 1)) & isi->width_flags)
-		return 0;
-	return -EINVAL;
+	return isi_try_fmt(isi, f, NULL);
 }
 
-
-static int isi_camera_get_formats(struct soc_camera_device *icd,
-				  unsigned int idx,
-				  struct soc_camera_format_xlate *xlate)
+static int isi_enum_fmt_vid_cap(struct file *file, void  *priv,
+				struct v4l2_fmtdesc *f)
 {
-	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
-	int formats = 0, ret, i, n;
-	/* sensor format */
-	struct v4l2_subdev_mbus_code_enum code = {
-		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
-		.index = idx,
-	};
-	/* soc camera host format */
-	const struct soc_mbus_pixelfmt *fmt;
+	struct atmel_isi *isi = video_drvdata(file);
 
-	ret = v4l2_subdev_call(sd, pad, enum_mbus_code, NULL, &code);
-	if (ret < 0)
-		/* No more formats */
-		return 0;
-
-	fmt = soc_mbus_get_fmtdesc(code.code);
-	if (!fmt) {
-		dev_err(icd->parent,
-			"Invalid format code #%u: %d\n", idx, code.code);
-		return 0;
-	}
+	if (f->index >= isi->num_user_formats)
+		return -EINVAL;
 
-	/* This also checks support for the requested bits-per-sample */
-	ret = isi_camera_try_bus_param(icd, fmt->bits_per_sample);
-	if (ret < 0) {
-		dev_err(icd->parent,
-			"Fail to try the bus parameters.\n");
-		return 0;
-	}
+	f->pixelformat = isi->user_formats[f->index]->fourcc;
+	return 0;
+}
 
-	switch (code.code) {
-	case MEDIA_BUS_FMT_UYVY8_2X8:
-	case MEDIA_BUS_FMT_VYUY8_2X8:
-	case MEDIA_BUS_FMT_YUYV8_2X8:
-	case MEDIA_BUS_FMT_YVYU8_2X8:
-		n = ARRAY_SIZE(isi_camera_formats);
-		formats += n;
-		for (i = 0; xlate && i < n; i++, xlate++) {
-			xlate->host_fmt	= &isi_camera_formats[i];
-			xlate->code	= code.code;
-			dev_dbg(icd->parent, "Providing format %s using code %d\n",
-				xlate->host_fmt->name, xlate->code);
-		}
-		break;
-	default:
-		if (!isi_camera_packing_supported(fmt))
-			return 0;
-		if (xlate)
-			dev_dbg(icd->parent,
-				"Providing format %s in pass-through mode\n",
-				fmt->name);
-	}
+static int isi_querycap(struct file *file, void *priv,
+			struct v4l2_capability *cap)
+{
+	strlcpy(cap->driver, "atmel-isi", sizeof(cap->driver));
+	strlcpy(cap->card, "Atmel Image Sensor Interface", sizeof(cap->card));
+	strlcpy(cap->bus_info, "platform:isi", sizeof(cap->bus_info));
+	return 0;
+}
 
-	/* Generic pass-through */
-	formats++;
-	if (xlate) {
-		xlate->host_fmt	= fmt;
-		xlate->code	= code.code;
-		xlate++;
-	}
+static int isi_enum_input(struct file *file, void *priv,
+			   struct v4l2_input *i)
+{
+	if (i->index != 0)
+		return -EINVAL;
 
-	return formats;
+	i->type = V4L2_INPUT_TYPE_CAMERA;
+	strlcpy(i->name, "Camera", sizeof(i->name));
+	return 0;
 }
 
-static int isi_camera_add_device(struct soc_camera_device *icd)
+static int isi_g_input(struct file *file, void *priv, unsigned int *i)
 {
-	dev_dbg(icd->parent, "Atmel ISI Camera driver attached to camera %d\n",
-		 icd->devnum);
+	*i = 0;
+	return 0;
+}
 
+static int isi_s_input(struct file *file, void *priv, unsigned int i)
+{
+	if (i > 0)
+		return -EINVAL;
 	return 0;
 }
 
-static void isi_camera_remove_device(struct soc_camera_device *icd)
+static int isi_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
 {
-	dev_dbg(icd->parent, "Atmel ISI Camera driver detached from camera %d\n",
-		 icd->devnum);
+	struct atmel_isi *isi = video_drvdata(file);
+
+	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	a->parm.capture.readbuffers = 2;
+	return v4l2_subdev_call(isi->entity.subdev, video, g_parm, a);
 }
 
-static unsigned int isi_camera_poll(struct file *file, poll_table *pt)
+static int isi_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
 {
-	struct soc_camera_device *icd = file->private_data;
+	struct atmel_isi *isi = video_drvdata(file);
+
+	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
 
-	return vb2_poll(&icd->vb2_vidq, file, pt);
+	a->parm.capture.readbuffers = 2;
+	return v4l2_subdev_call(isi->entity.subdev, video, s_parm, a);
 }
 
-static int isi_camera_querycap(struct soc_camera_host *ici,
-			       struct v4l2_capability *cap)
+static int isi_enum_framesizes(struct file *file, void *fh,
+			       struct v4l2_frmsizeenum *fsize)
 {
-	strcpy(cap->driver, "atmel-isi");
-	strcpy(cap->card, "Atmel Image Sensor Interface");
-	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
-	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
+	struct atmel_isi *isi = video_drvdata(file);
+	const struct isi_format *isi_fmt;
+	struct v4l2_subdev_frame_size_enum fse = {
+		.index = fsize->index,
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
+	int ret;
+
+	isi_fmt = find_format_by_fourcc(isi, fsize->pixel_format);
+	if (!isi_fmt)
+		return -EINVAL;
+
+	fse.code = isi_fmt->mbus_code;
+
+	ret = v4l2_subdev_call(isi->entity.subdev, pad, enum_frame_size,
+			       NULL, &fse);
+	if (ret)
+		return ret;
+
+	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+	fsize->discrete.width = fse.max_width;
+	fsize->discrete.height = fse.max_height;
 
 	return 0;
 }
 
-static int isi_camera_set_bus_param(struct soc_camera_device *icd)
+static int isi_enum_frameintervals(struct file *file, void *fh,
+				    struct v4l2_frmivalenum *fival)
 {
-	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct atmel_isi *isi = ici->priv;
-	struct v4l2_mbus_config cfg = {.type = V4L2_MBUS_PARALLEL,};
-	unsigned long common_flags;
+	struct atmel_isi *isi = video_drvdata(file);
+	const struct isi_format *isi_fmt;
+	struct v4l2_subdev_frame_interval_enum fie = {
+		.index = fival->index,
+		.width = fival->width,
+		.height = fival->height,
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
 	int ret;
-	u32 cfg1 = 0;
 
-	ret = v4l2_subdev_call(sd, video, g_mbus_config, &cfg);
-	if (!ret) {
-		common_flags = soc_mbus_config_compatible(&cfg,
-							  ISI_BUS_PARAM);
-		if (!common_flags) {
-			dev_warn(icd->parent,
-				 "Flags incompatible: camera 0x%x, host 0x%x\n",
-				 cfg.flags, ISI_BUS_PARAM);
-			return -EINVAL;
-		}
-	} else if (ret != -ENOIOCTLCMD) {
+	isi_fmt = find_format_by_fourcc(isi, fival->pixel_format);
+	if (!isi_fmt)
+		return -EINVAL;
+
+	fie.code = isi_fmt->mbus_code;
+
+	ret = v4l2_subdev_call(isi->entity.subdev, pad,
+			       enum_frame_interval, NULL, &fie);
+	if (ret)
 		return ret;
-	} else {
-		common_flags = ISI_BUS_PARAM;
-	}
-	dev_dbg(icd->parent, "Flags cam: 0x%x host: 0x%x common: 0x%lx\n",
-		cfg.flags, ISI_BUS_PARAM, common_flags);
-
-	/* Make choises, based on platform preferences */
-	if ((common_flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) &&
-	    (common_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)) {
-		if (isi->pdata.hsync_act_low)
-			common_flags &= ~V4L2_MBUS_HSYNC_ACTIVE_HIGH;
-		else
-			common_flags &= ~V4L2_MBUS_HSYNC_ACTIVE_LOW;
-	}
 
-	if ((common_flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) &&
-	    (common_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)) {
-		if (isi->pdata.vsync_act_low)
-			common_flags &= ~V4L2_MBUS_VSYNC_ACTIVE_HIGH;
-		else
-			common_flags &= ~V4L2_MBUS_VSYNC_ACTIVE_LOW;
-	}
+	fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
+	fival->discrete = fie.interval;
 
-	if ((common_flags & V4L2_MBUS_PCLK_SAMPLE_RISING) &&
-	    (common_flags & V4L2_MBUS_PCLK_SAMPLE_FALLING)) {
-		if (isi->pdata.pclk_act_falling)
-			common_flags &= ~V4L2_MBUS_PCLK_SAMPLE_RISING;
-		else
-			common_flags &= ~V4L2_MBUS_PCLK_SAMPLE_FALLING;
-	}
+	return 0;
+}
 
-	cfg.flags = common_flags;
-	ret = v4l2_subdev_call(sd, video, s_mbus_config, &cfg);
-	if (ret < 0 && ret != -ENOIOCTLCMD) {
-		dev_dbg(icd->parent, "camera s_mbus_config(0x%lx) returned %d\n",
-			common_flags, ret);
-		return ret;
-	}
+static void isi_camera_set_bus_param(struct atmel_isi *isi)
+{
+	u32 cfg1 = 0;
 
 	/* set bus param for ISI */
-	if (common_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)
+	if (isi->pdata.hsync_act_low)
 		cfg1 |= ISI_CFG1_HSYNC_POL_ACTIVE_LOW;
-	if (common_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)
+	if (isi->pdata.vsync_act_low)
 		cfg1 |= ISI_CFG1_VSYNC_POL_ACTIVE_LOW;
-	if (common_flags & V4L2_MBUS_PCLK_SAMPLE_FALLING)
+	if (isi->pdata.pclk_act_falling)
 		cfg1 |= ISI_CFG1_PIXCLK_POL_ACTIVE_FALLING;
-
-	dev_dbg(icd->parent, "vsync active %s, hsync active %s, sampling on pix clock %s edge\n",
-		common_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW ? "low" : "high",
-		common_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW ? "low" : "high",
-		common_flags & V4L2_MBUS_PCLK_SAMPLE_FALLING ? "falling" : "rising");
-
 	if (isi->pdata.has_emb_sync)
 		cfg1 |= ISI_CFG1_EMB_SYNC;
 	if (isi->pdata.full_mode)
@@ -930,50 +804,19 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 	cfg1 |= ISI_CFG1_THMASK_BEATS_16;
 
 	/* Enable PM and peripheral clock before operate isi registers */
-	pm_runtime_get_sync(ici->v4l2_dev.dev);
+	pm_runtime_get_sync(isi->dev);
 
 	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
 	isi_writel(isi, ISI_CFG1, cfg1);
 
-	pm_runtime_put(ici->v4l2_dev.dev);
-
-	return 0;
+	pm_runtime_put(isi->dev);
 }
 
-static struct soc_camera_host_ops isi_soc_camera_host_ops = {
-	.owner		= THIS_MODULE,
-	.add		= isi_camera_add_device,
-	.remove		= isi_camera_remove_device,
-	.set_fmt	= isi_camera_set_fmt,
-	.try_fmt	= isi_camera_try_fmt,
-	.get_formats	= isi_camera_get_formats,
-	.init_videobuf2	= isi_camera_init_videobuf,
-	.poll		= isi_camera_poll,
-	.querycap	= isi_camera_querycap,
-	.set_bus_param	= isi_camera_set_bus_param,
-};
-
 /* -----------------------------------------------------------------------*/
-static int atmel_isi_remove(struct platform_device *pdev)
-{
-	struct soc_camera_host *soc_host = to_soc_camera_host(&pdev->dev);
-	struct atmel_isi *isi = container_of(soc_host,
-					struct atmel_isi, soc_host);
-
-	soc_camera_host_unregister(soc_host);
-	dma_free_coherent(&pdev->dev,
-			sizeof(struct fbd) * MAX_BUFFER_NUM,
-			isi->p_fb_descriptors,
-			isi->fb_descriptors_phys);
-	pm_runtime_disable(&pdev->dev);
-
-	return 0;
-}
-
 static int atmel_isi_parse_dt(struct atmel_isi *isi,
 			struct platform_device *pdev)
 {
-	struct device_node *np= pdev->dev.of_node;
+	struct device_node *np = pdev->dev.of_node;
 	struct v4l2_of_endpoint ep;
 	int err;
 
@@ -1021,13 +864,349 @@ static int atmel_isi_parse_dt(struct atmel_isi *isi,
 	return 0;
 }
 
+static int isi_open(struct file *file)
+{
+	struct atmel_isi *isi = video_drvdata(file);
+	struct v4l2_subdev *sd = isi->entity.subdev;
+	int ret;
+
+	if (mutex_lock_interruptible(&isi->lock))
+		return -ERESTARTSYS;
+
+	ret = v4l2_fh_open(file);
+	if (ret < 0)
+		goto unlock;
+
+	if (!v4l2_fh_is_singular_file(file))
+		goto fh_rel;
+
+	ret = v4l2_subdev_call(sd, core, s_power, 1);
+	if (ret < 0 && ret != -ENOIOCTLCMD)
+		goto fh_rel;
+
+	ret = isi_set_fmt(isi, &isi->fmt);
+	if (ret)
+		v4l2_subdev_call(sd, core, s_power, 0);
+fh_rel:
+	if (ret)
+		v4l2_fh_release(file);
+unlock:
+	mutex_unlock(&isi->lock);
+	return ret;
+}
+
+static int isi_release(struct file *file)
+{
+	struct atmel_isi *isi = video_drvdata(file);
+	struct v4l2_subdev *sd = isi->entity.subdev;
+	bool fh_singular;
+	int ret;
+
+	mutex_lock(&isi->lock);
+
+	fh_singular = v4l2_fh_is_singular_file(file);
+
+	ret = _vb2_fop_release(file, NULL);
+
+	if (fh_singular)
+		v4l2_subdev_call(sd, core, s_power, 0);
+
+	mutex_unlock(&isi->lock);
+
+	return ret;
+}
+
+static const struct v4l2_ioctl_ops isi_ioctl_ops = {
+	.vidioc_querycap		= isi_querycap,
+
+	.vidioc_try_fmt_vid_cap		= isi_try_fmt_vid_cap,
+	.vidioc_g_fmt_vid_cap		= isi_g_fmt_vid_cap,
+	.vidioc_s_fmt_vid_cap		= isi_s_fmt_vid_cap,
+	.vidioc_enum_fmt_vid_cap	= isi_enum_fmt_vid_cap,
+
+	.vidioc_enum_input		= isi_enum_input,
+	.vidioc_g_input			= isi_g_input,
+	.vidioc_s_input			= isi_s_input,
+
+	.vidioc_g_parm			= isi_g_parm,
+	.vidioc_s_parm			= isi_s_parm,
+	.vidioc_enum_framesizes		= isi_enum_framesizes,
+	.vidioc_enum_frameintervals	= isi_enum_frameintervals,
+
+	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
+	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
+	.vidioc_querybuf		= vb2_ioctl_querybuf,
+	.vidioc_qbuf			= vb2_ioctl_qbuf,
+	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
+	.vidioc_expbuf			= vb2_ioctl_expbuf,
+	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
+	.vidioc_streamon		= vb2_ioctl_streamon,
+	.vidioc_streamoff		= vb2_ioctl_streamoff,
+
+	.vidioc_log_status		= v4l2_ctrl_log_status,
+	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,
+	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
+};
+
+static const struct v4l2_file_operations isi_fops = {
+	.owner		= THIS_MODULE,
+	.unlocked_ioctl	= video_ioctl2,
+	.open		= isi_open,
+	.release	= isi_release,
+	.poll		= vb2_fop_poll,
+	.mmap		= vb2_fop_mmap,
+	.read		= vb2_fop_read,
+};
+
+static int isi_set_default_fmt(struct atmel_isi *isi)
+{
+	struct v4l2_format f = {
+		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+		.fmt.pix = {
+			.width		= VGA_WIDTH,
+			.height		= VGA_HEIGHT,
+			.field		= V4L2_FIELD_NONE,
+			.pixelformat	= isi->user_formats[0]->fourcc,
+		},
+	};
+	int ret;
+
+	ret = isi_try_fmt(isi, &f, NULL);
+	if (ret)
+		return ret;
+	isi->current_fmt = isi->user_formats[0];
+	isi->fmt = f;
+	return 0;
+}
+
+static struct isi_format *find_format_by_code(unsigned int code, int *index)
+{
+	struct isi_format *fmt = &isi_formats[0];
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(isi_formats); i++) {
+		if (fmt->mbus_code == code && !fmt->support && !fmt->skip) {
+			*index = i;
+			return fmt;
+		}
+
+		fmt++;
+	}
+
+	return NULL;
+}
+
+static int isi_formats_init(struct atmel_isi *isi)
+{
+	struct isi_format *fmt;
+	struct v4l2_subdev *subdev = isi->entity.subdev;
+	int num_fmts = 0, i, j;
+	struct v4l2_subdev_mbus_code_enum mbus_code = {
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
+
+	fmt = &isi_formats[0];
+
+	while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
+	       NULL, &mbus_code)) {
+		fmt = find_format_by_code(mbus_code.code, &i);
+		if (!fmt) {
+			mbus_code.index++;
+			continue;
+		}
+
+		fmt->support = true;
+		for (i = 0; i < ARRAY_SIZE(isi_formats); i++)
+			if (isi_formats[i].fourcc == fmt->fourcc &&
+			    !isi_formats[i].support)
+				isi_formats[i].skip = true;
+		num_fmts++;
+	}
+
+	if (!num_fmts)
+		return -ENXIO;
+
+	isi->num_user_formats = num_fmts;
+	isi->user_formats = devm_kcalloc(isi->dev,
+					 num_fmts, sizeof(struct isi_format *),
+					 GFP_KERNEL);
+	if (!isi->user_formats) {
+		v4l2_err(&isi->v4l2_dev, "could not allocate memory\n");
+		return -ENOMEM;
+	}
+
+	fmt = &isi_formats[0];
+	for (i = 0, j = 0; i < ARRAY_SIZE(isi_formats); i++) {
+		if (fmt->support)
+			isi->user_formats[j++] = fmt;
+
+		fmt++;
+	}
+	isi->current_fmt = isi->user_formats[0];
+
+	return 0;
+}
+
+static int isi_graph_notify_complete(struct v4l2_async_notifier *notifier)
+{
+	struct atmel_isi *isi = notifier_to_isi(notifier);
+	int ret;
+
+	isi->vdev->ctrl_handler	= isi->entity.subdev->ctrl_handler;
+	ret = isi_formats_init(isi);
+	if (ret) {
+		dev_err(isi->dev, "No supported mediabus format found\n");
+		return ret;
+	}
+	isi_camera_set_bus_param(isi);
+
+	ret = isi_set_default_fmt(isi);
+	if (ret) {
+		dev_err(isi->dev, "Could not set default format\n");
+		return ret;
+	}
+
+	ret = video_register_device(isi->vdev, VFL_TYPE_GRABBER, -1);
+	if (ret) {
+		dev_err(isi->dev, "Failed to register video device\n");
+		return ret;
+	}
+
+	dev_info(isi->dev, "Device registered as %s\n",
+		 video_device_node_name(isi->vdev));
+	return 0;
+}
+
+static void isi_graph_notify_unbind(struct v4l2_async_notifier *notifier,
+				     struct v4l2_subdev *sd,
+				     struct v4l2_async_subdev *asd)
+{
+	struct atmel_isi *isi = notifier_to_isi(notifier);
+
+	dev_info(isi->dev, "Removing %s\n",
+		 video_device_node_name(isi->vdev));
+
+	/* Checks internaly if vdev have been init or not */
+	video_unregister_device(isi->vdev);
+}
+
+static int isi_graph_notify_bound(struct v4l2_async_notifier *notifier,
+				   struct v4l2_subdev *subdev,
+				   struct v4l2_async_subdev *asd)
+{
+	struct atmel_isi *isi = notifier_to_isi(notifier);
+
+	dev_dbg(isi->dev, "subdev %s bound\n", subdev->name);
+
+	isi->entity.subdev = subdev;
+
+	return 0;
+}
+
+static int isi_graph_parse(struct atmel_isi *isi,
+			    struct device_node *node)
+{
+	struct device_node *remote;
+	struct device_node *ep = NULL;
+	struct device_node *next;
+	int ret = 0;
+
+	while (1) {
+		next = of_graph_get_next_endpoint(node, ep);
+		if (!next)
+			break;
+
+		of_node_put(ep);
+		ep = next;
+
+		remote = of_graph_get_remote_port_parent(ep);
+		if (!remote) {
+			ret = -EINVAL;
+			break;
+		}
+
+		/* Skip entities that we have already processed. */
+		if (remote == isi->dev->of_node) {
+			of_node_put(remote);
+			continue;
+		}
+
+		/* Remote node to connect */
+		if (!isi->entity.node) {
+			isi->entity.node = remote;
+			isi->entity.asd.match_type = V4L2_ASYNC_MATCH_OF;
+			isi->entity.asd.match.of.node = remote;
+			ret++;
+		}
+	}
+
+	of_node_put(ep);
+
+	return ret;
+}
+
+static int isi_graph_init(struct atmel_isi *isi)
+{
+	struct v4l2_async_subdev **subdevs = NULL;
+	int ret;
+
+	/* Parse the graph to extract a list of subdevice DT nodes. */
+	ret = isi_graph_parse(isi, isi->dev->of_node);
+	if (ret < 0) {
+		dev_err(isi->dev, "Graph parsing failed\n");
+		goto done;
+	}
+
+	if (!ret) {
+		dev_err(isi->dev, "No subdev found in graph\n");
+		goto done;
+	}
+
+	if (ret != 1) {
+		dev_err(isi->dev, "More then one subdev found in graph\n");
+		goto done;
+	}
+
+	/* Register the subdevices notifier. */
+	subdevs = devm_kzalloc(isi->dev, sizeof(*subdevs), GFP_KERNEL);
+	if (subdevs == NULL) {
+		ret = -ENOMEM;
+		goto done;
+	}
+
+	subdevs[0] = &isi->entity.asd;
+
+	isi->notifier.subdevs = subdevs;
+	isi->notifier.num_subdevs = 1;
+	isi->notifier.bound = isi_graph_notify_bound;
+	isi->notifier.unbind = isi_graph_notify_unbind;
+	isi->notifier.complete = isi_graph_notify_complete;
+
+	ret = v4l2_async_notifier_register(&isi->v4l2_dev, &isi->notifier);
+	if (ret < 0) {
+		dev_err(isi->dev, "Notifier registration failed\n");
+		goto done;
+	}
+
+	ret = 0;
+
+done:
+	if (ret < 0) {
+		v4l2_async_notifier_unregister(&isi->notifier);
+		of_node_put(isi->entity.node);
+	}
+
+	return ret;
+}
+
+
 static int atmel_isi_probe(struct platform_device *pdev)
 {
 	int irq;
 	struct atmel_isi *isi;
+	struct vb2_queue *q;
 	struct resource *regs;
 	int ret, i;
-	struct soc_camera_host *soc_host;
 
 	isi = devm_kzalloc(&pdev->dev, sizeof(struct atmel_isi), GFP_KERNEL);
 	if (!isi) {
@@ -1044,20 +1223,65 @@ static int atmel_isi_probe(struct platform_device *pdev)
 		return ret;
 
 	isi->active = NULL;
-	spin_lock_init(&isi->lock);
+	isi->dev = &pdev->dev;
+	mutex_init(&isi->lock);
+	spin_lock_init(&isi->irqlock);
 	INIT_LIST_HEAD(&isi->video_buffer_list);
 	INIT_LIST_HEAD(&isi->dma_desc_head);
 
+	q = &isi->queue;
+
+	/* Initialize the top-level structure */
+	ret = v4l2_device_register(&pdev->dev, &isi->v4l2_dev);
+	if (ret)
+		return ret;
+
+	isi->vdev = video_device_alloc();
+	if (isi->vdev == NULL) {
+		ret = -ENOMEM;
+		goto err_vdev_alloc;
+	}
+
+	/* video node */
+	isi->vdev->fops = &isi_fops;
+	isi->vdev->v4l2_dev = &isi->v4l2_dev;
+	isi->vdev->queue = &isi->queue;
+	strlcpy(isi->vdev->name, KBUILD_MODNAME, sizeof(isi->vdev->name));
+	isi->vdev->release = video_device_release;
+	isi->vdev->ioctl_ops = &isi_ioctl_ops;
+	isi->vdev->lock = &isi->lock;
+	isi->vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
+		V4L2_CAP_READWRITE;
+	video_set_drvdata(isi->vdev, isi);
+
+	/* buffer queue */
+	q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	q->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
+	q->lock = &isi->lock;
+	q->drv_priv = isi;
+	q->buf_struct_size = sizeof(struct frame_buffer);
+	q->ops = &isi_video_qops;
+	q->mem_ops = &vb2_dma_contig_memops;
+	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	q->min_buffers_needed = 2;
+	q->dev = &pdev->dev;
+
+	ret = vb2_queue_init(q);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to initialize VB2 queue\n");
+		goto err_vb2_queue;
+	}
 	isi->p_fb_descriptors = dma_alloc_coherent(&pdev->dev,
-				sizeof(struct fbd) * MAX_BUFFER_NUM,
+				sizeof(struct fbd) * VIDEO_MAX_FRAME,
 				&isi->fb_descriptors_phys,
 				GFP_KERNEL);
 	if (!isi->p_fb_descriptors) {
 		dev_err(&pdev->dev, "Can't allocate descriptors!\n");
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto err_dma_alloc;
 	}
 
-	for (i = 0; i < MAX_BUFFER_NUM; i++) {
+	for (i = 0; i < VIDEO_MAX_FRAME; i++) {
 		isi->dma_desc[i].p_fbd = isi->p_fb_descriptors + i;
 		isi->dma_desc[i].fbd_phys = isi->fb_descriptors_phys +
 					i * sizeof(struct fbd);
@@ -1089,41 +1313,49 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	}
 	isi->irq = irq;
 
-	soc_host		= &isi->soc_host;
-	soc_host->drv_name	= "isi-camera";
-	soc_host->ops		= &isi_soc_camera_host_ops;
-	soc_host->priv		= isi;
-	soc_host->v4l2_dev.dev	= &pdev->dev;
-	soc_host->nr		= pdev->id;
+	ret = isi_graph_init(isi);
+	if (ret < 0)
+		goto err_req_irq;
 
 	pm_suspend_ignore_children(&pdev->dev, true);
 	pm_runtime_enable(&pdev->dev);
-
-	ret = soc_camera_host_register(soc_host);
-	if (ret) {
-		dev_err(&pdev->dev, "Unable to register soc camera host\n");
-		goto err_register_soc_camera_host;
-	}
+	platform_set_drvdata(pdev, isi);
 	return 0;
 
-err_register_soc_camera_host:
-	pm_runtime_disable(&pdev->dev);
 err_req_irq:
 err_ioremap:
 	dma_free_coherent(&pdev->dev,
-			sizeof(struct fbd) * MAX_BUFFER_NUM,
+			sizeof(struct fbd) * VIDEO_MAX_FRAME,
 			isi->p_fb_descriptors,
 			isi->fb_descriptors_phys);
+err_dma_alloc:
+err_vb2_queue:
+	video_device_release(isi->vdev);
+err_vdev_alloc:
+	v4l2_device_unregister(&isi->v4l2_dev);
 
 	return ret;
 }
 
+static int atmel_isi_remove(struct platform_device *pdev)
+{
+	struct atmel_isi *isi = platform_get_drvdata(pdev);
+
+	dma_free_coherent(&pdev->dev,
+			sizeof(struct fbd) * VIDEO_MAX_FRAME,
+			isi->p_fb_descriptors,
+			isi->fb_descriptors_phys);
+	pm_runtime_disable(&pdev->dev);
+	v4l2_async_notifier_unregister(&isi->notifier);
+	v4l2_device_unregister(&isi->v4l2_dev);
+
+	return 0;
+}
+
 #ifdef CONFIG_PM
 static int atmel_isi_runtime_suspend(struct device *dev)
 {
-	struct soc_camera_host *soc_host = to_soc_camera_host(dev);
-	struct atmel_isi *isi = container_of(soc_host,
-					struct atmel_isi, soc_host);
+	struct atmel_isi *isi = dev_get_drvdata(dev);
 
 	clk_disable_unprepare(isi->pclk);
 
@@ -1131,9 +1363,7 @@ static int atmel_isi_runtime_suspend(struct device *dev)
 }
 static int atmel_isi_runtime_resume(struct device *dev)
 {
-	struct soc_camera_host *soc_host = to_soc_camera_host(dev);
-	struct atmel_isi *isi = container_of(soc_host,
-					struct atmel_isi, soc_host);
+	struct atmel_isi *isi = dev_get_drvdata(dev);
 
 	return clk_prepare_enable(isi->pclk);
 }
@@ -1151,15 +1381,16 @@ static const struct of_device_id atmel_isi_of_match[] = {
 MODULE_DEVICE_TABLE(of, atmel_isi_of_match);
 
 static struct platform_driver atmel_isi_driver = {
-	.remove		= atmel_isi_remove,
 	.driver		= {
 		.name = "atmel_isi",
 		.of_match_table = of_match_ptr(atmel_isi_of_match),
 		.pm	= &atmel_isi_dev_pm_ops,
 	},
+	.probe		= atmel_isi_probe,
+	.remove		= atmel_isi_remove,
 };
 
-module_platform_driver_probe(atmel_isi_driver, atmel_isi_probe);
+module_platform_driver(atmel_isi_driver);
 
 MODULE_AUTHOR("Josh Wu <josh.wu-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>");
 MODULE_DESCRIPTION("The V4L2 driver for Atmel Linux");
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 08/15] atmel-isi: move out of soc_camera to atmel

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Move this out of the soc_camera directory into the atmel directory
where it belongs.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/platform/Makefile                          |  1 +
 drivers/media/platform/atmel/Kconfig                     | 11 ++++++++++-
 drivers/media/platform/atmel/Makefile                    |  1 +
 drivers/media/platform/{soc_camera => atmel}/atmel-isi.c |  0
 drivers/media/platform/{soc_camera => atmel}/atmel-isi.h |  0
 drivers/media/platform/soc_camera/Kconfig                | 10 ----------
 drivers/media/platform/soc_camera/Makefile               |  1 -
 7 files changed, 12 insertions(+), 12 deletions(-)
 rename drivers/media/platform/{soc_camera => atmel}/atmel-isi.c (100%)
 rename drivers/media/platform/{soc_camera => atmel}/atmel-isi.h (100%)

diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 8959f6e6692a..c491731f5909 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_VIDEO_XILINX)		+= xilinx/
 obj-$(CONFIG_VIDEO_RCAR_VIN)		+= rcar-vin/
 
 obj-$(CONFIG_VIDEO_ATMEL_ISC)		+= atmel/
+obj-$(CONFIG_VIDEO_ATMEL_ISI)		+= atmel/
 
 ccflags-y += -I$(srctree)/drivers/media/i2c
 
diff --git a/drivers/media/platform/atmel/Kconfig b/drivers/media/platform/atmel/Kconfig
index 867dca22a473..9bd0f19b127f 100644
--- a/drivers/media/platform/atmel/Kconfig
+++ b/drivers/media/platform/atmel/Kconfig
@@ -6,4 +6,13 @@ config VIDEO_ATMEL_ISC
 	select REGMAP_MMIO
 	help
 	   This module makes the ATMEL Image Sensor Controller available
-	   as a v4l2 device.
\ No newline at end of file
+	   as a v4l2 device.
+
+config VIDEO_ATMEL_ISI
+	tristate "ATMEL Image Sensor Interface (ISI) support"
+	depends on VIDEO_V4L2 && OF && HAS_DMA
+	depends on ARCH_AT91 || COMPILE_TEST
+	select VIDEOBUF2_DMA_CONTIG
+	---help---
+	  This module makes the ATMEL Image Sensor Interface available
+	  as a v4l2 device.
diff --git a/drivers/media/platform/atmel/Makefile b/drivers/media/platform/atmel/Makefile
index 9d7c999d434d..27000d099a5e 100644
--- a/drivers/media/platform/atmel/Makefile
+++ b/drivers/media/platform/atmel/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel-isc.o
+obj-$(CONFIG_VIDEO_ATMEL_ISI) += atmel-isi.o
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
similarity index 100%
rename from drivers/media/platform/soc_camera/atmel-isi.c
rename to drivers/media/platform/atmel/atmel-isi.c
diff --git a/drivers/media/platform/soc_camera/atmel-isi.h b/drivers/media/platform/atmel/atmel-isi.h
similarity index 100%
rename from drivers/media/platform/soc_camera/atmel-isi.h
rename to drivers/media/platform/atmel/atmel-isi.h
diff --git a/drivers/media/platform/soc_camera/Kconfig b/drivers/media/platform/soc_camera/Kconfig
index a37ec91b026e..0c581aa1b18a 100644
--- a/drivers/media/platform/soc_camera/Kconfig
+++ b/drivers/media/platform/soc_camera/Kconfig
@@ -26,13 +26,3 @@ config VIDEO_SH_MOBILE_CEU
 	select SOC_CAMERA_SCALE_CROP
 	---help---
 	  This is a v4l2 driver for the SuperH Mobile CEU Interface
-
-config VIDEO_ATMEL_ISI
-	tristate "ATMEL Image Sensor Interface (ISI) support"
-	depends on VIDEO_V4L2 && OF && HAS_DMA
-	depends on ARCH_AT91 || COMPILE_TEST
-	select VIDEOBUF2_DMA_CONTIG
-	---help---
-	  This module makes the ATMEL Image Sensor Interface available
-	  as a v4l2 device.
-
diff --git a/drivers/media/platform/soc_camera/Makefile b/drivers/media/platform/soc_camera/Makefile
index 7633a0f2f66f..07a451e8b228 100644
--- a/drivers/media/platform/soc_camera/Makefile
+++ b/drivers/media/platform/soc_camera/Makefile
@@ -6,5 +6,4 @@ obj-$(CONFIG_SOC_CAMERA_SCALE_CROP)	+= soc_scale_crop.o
 obj-$(CONFIG_SOC_CAMERA_PLATFORM)	+= soc_camera_platform.o
 
 # soc-camera host drivers have to be linked after camera drivers
-obj-$(CONFIG_VIDEO_ATMEL_ISI)		+= atmel-isi.o
 obj-$(CONFIG_VIDEO_SH_MOBILE_CEU)	+= sh_mobile_ceu_camera.o
-- 
2.11.0

[PATCHv3 09/15] ov2640: update bindings

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

Update the bindings for this device based on a working DT example.

Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 .../devicetree/bindings/media/i2c/ov2640.txt       | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2640.txt b/Documentation/devicetree/bindings/media/i2c/ov2640.txt
index c429b5bdcaa0..48d97c9f3371 100644
--- a/Documentation/devicetree/bindings/media/i2c/ov2640.txt
+++ b/Documentation/devicetree/bindings/media/i2c/ov2640.txt
@@ -1,8 +1,8 @@
 * Omnivision OV2640 CMOS sensor
 
-The Omnivision OV2640 sensor support multiple resolutions output, such as
-CIF, SVGA, UXGA. It also can support YUV422/420, RGB565/555 or raw RGB
-output format.
+The Omnivision OV2640 sensor supports multiple resolutions output, such as
+CIF, SVGA, UXGA. It also can support the YUV422/420, RGB565/555 or raw RGB
+output formats.
 
 Required Properties:
 - compatible: should be "ovti,ov2640"
@@ -20,20 +20,16 @@ Documentation/devicetree/bindings/media/video-interfaces.txt.
 Example:
 
 	i2c1: i2c@f0018000 {
-		ov2640: camera@0x30 {
+		ov2640: camera@30 {
 			compatible = "ovti,ov2640";
 			reg = <0x30>;
-
 			pinctrl-names = "default";
-			pinctrl-0 = <&pinctrl_pck1 &pinctrl_ov2640_pwdn &pinctrl_ov2640_resetb>;
-
-			resetb-gpios = <&pioE 24 GPIO_ACTIVE_LOW>;
-			pwdn-gpios = <&pioE 29 GPIO_ACTIVE_HIGH>;
-
-			clocks = <&pck1>;
+			pinctrl-0 = <&pinctrl_pck0_as_isi_mck &pinctrl_sensor_power &pinctrl_sensor_reset>;
+			resetb-gpios = <&pioE 11 GPIO_ACTIVE_LOW>;
+			pwdn-gpios = <&pioE 13 GPIO_ACTIVE_HIGH>;
+			clocks = <&pck0>;
 			clock-names = "xvclk";
-
-			assigned-clocks = <&pck1>;
+			assigned-clocks = <&pck0>;
 			assigned-clock-rates = <25000000>;
 
 			port {
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 10/15] ov2640: convert from soc-camera to a standard subdev sensor driver.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Convert ov2640 to a standard subdev driver. The soc-camera driver no longer
uses this driver, so it can safely be converted.

Note: the s_power op has been dropped: this never worked. When the last open()
is closed, then the power is turned off, and when it is opened again the power
is turned on again, but the old state isn't restored.

Someone else can figure out in the future how to get this working correctly,
but I don't want to spend more time on this.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/Kconfig                   | 11 ++++
 drivers/media/i2c/Makefile                  |  1 +
 drivers/media/i2c/{soc_camera => }/ov2640.c | 89 +++++------------------------
 drivers/media/i2c/soc_camera/Kconfig        |  6 --
 drivers/media/i2c/soc_camera/Makefile       |  1 -
 5 files changed, 27 insertions(+), 81 deletions(-)
 rename drivers/media/i2c/{soc_camera => }/ov2640.c (94%)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cee1dae6e014..db2c63f592c5 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -520,6 +520,17 @@ config VIDEO_APTINA_PLL
 config VIDEO_SMIAPP_PLL
 	tristate
 
+config VIDEO_OV2640
+	tristate "OmniVision OV2640 sensor support"
+	depends on VIDEO_V4L2 && I2C && GPIOLIB
+	depends on MEDIA_CAMERA_SUPPORT
+	help
+	  This is a Video4Linux2 sensor-level driver for the OmniVision
+	  OV2640 camera.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ov2640.
+
 config VIDEO_OV2659
 	tristate "OmniVision OV2659 sensor support"
 	depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 5bc7bbeb5499..50af1e11c85a 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
 obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
+obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
 obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
 obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
 obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
diff --git a/drivers/media/i2c/soc_camera/ov2640.c b/drivers/media/i2c/ov2640.c
similarity index 94%
rename from drivers/media/i2c/soc_camera/ov2640.c
rename to drivers/media/i2c/ov2640.c
index b9a0069f5b33..83f88efbce69 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -24,8 +24,8 @@
 #include <linux/v4l2-mediabus.h>
 #include <linux/videodev2.h>
 
-#include <media/soc_camera.h>
 #include <media/v4l2-clk.h>
+#include <media/v4l2-device.h>
 #include <media/v4l2-subdev.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-image-sizes.h>
@@ -287,7 +287,6 @@ struct ov2640_priv {
 	struct v4l2_clk			*clk;
 	const struct ov2640_win_size	*win;
 
-	struct soc_camera_subdev_desc	ssdd_dt;
 	struct gpio_desc *resetb_gpio;
 	struct gpio_desc *pwdn_gpio;
 };
@@ -677,13 +676,8 @@ static int ov2640_reset(struct i2c_client *client)
 }
 
 /*
- * soc_camera_ops functions
+ * functions
  */
-static int ov2640_s_stream(struct v4l2_subdev *sd, int enable)
-{
-	return 0;
-}
-
 static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct v4l2_subdev *sd =
@@ -744,10 +738,16 @@ static int ov2640_s_register(struct v4l2_subdev *sd,
 static int ov2640_s_power(struct v4l2_subdev *sd, int on)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
 	struct ov2640_priv *priv = to_ov2640(client);
 
-	return soc_camera_set_power(&client->dev, ssdd, priv->clk, on);
+	gpiod_direction_output(priv->pwdn_gpio, !on);
+	if (on && priv->resetb_gpio) {
+		/* Active the resetb pin to perform a reset pulse */
+		gpiod_direction_output(priv->resetb_gpio, 1);
+		usleep_range(3000, 5000);
+		gpiod_direction_output(priv->resetb_gpio, 0);
+	}
+	return 0;
 }
 
 /* Select the nearest higher resolution for capture */
@@ -994,26 +994,6 @@ static struct v4l2_subdev_core_ops ov2640_subdev_core_ops = {
 	.s_power	= ov2640_s_power,
 };
 
-static int ov2640_g_mbus_config(struct v4l2_subdev *sd,
-				struct v4l2_mbus_config *cfg)
-{
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
-
-	cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
-		V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
-		V4L2_MBUS_DATA_ACTIVE_HIGH;
-	cfg->type = V4L2_MBUS_PARALLEL;
-	cfg->flags = soc_camera_apply_board_flags(ssdd, cfg);
-
-	return 0;
-}
-
-static struct v4l2_subdev_video_ops ov2640_subdev_video_ops = {
-	.s_stream	= ov2640_s_stream,
-	.g_mbus_config	= ov2640_g_mbus_config,
-};
-
 static const struct v4l2_subdev_pad_ops ov2640_subdev_pad_ops = {
 	.enum_mbus_code = ov2640_enum_mbus_code,
 	.get_selection	= ov2640_get_selection,
@@ -1023,40 +1003,9 @@ static const struct v4l2_subdev_pad_ops ov2640_subdev_pad_ops = {
 
 static struct v4l2_subdev_ops ov2640_subdev_ops = {
 	.core	= &ov2640_subdev_core_ops,
-	.video	= &ov2640_subdev_video_ops,
 	.pad	= &ov2640_subdev_pad_ops,
 };
 
-/* OF probe functions */
-static int ov2640_hw_power(struct device *dev, int on)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct ov2640_priv *priv = to_ov2640(client);
-
-	dev_dbg(&client->dev, "%s: %s the camera\n",
-			__func__, on ? "ENABLE" : "DISABLE");
-
-	if (priv->pwdn_gpio)
-		gpiod_direction_output(priv->pwdn_gpio, !on);
-
-	return 0;
-}
-
-static int ov2640_hw_reset(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct ov2640_priv *priv = to_ov2640(client);
-
-	if (priv->resetb_gpio) {
-		/* Active the resetb pin to perform a reset pulse */
-		gpiod_direction_output(priv->resetb_gpio, 1);
-		usleep_range(3000, 5000);
-		gpiod_direction_output(priv->resetb_gpio, 0);
-	}
-
-	return 0;
-}
-
 static int ov2640_probe_dt(struct i2c_client *client,
 		struct ov2640_priv *priv)
 {
@@ -1076,11 +1025,6 @@ static int ov2640_probe_dt(struct i2c_client *client,
 	else if (IS_ERR(priv->pwdn_gpio))
 		return PTR_ERR(priv->pwdn_gpio);
 
-	/* Initialize the soc_camera_subdev_desc */
-	priv->ssdd_dt.power = ov2640_hw_power;
-	priv->ssdd_dt.reset = ov2640_hw_reset;
-	client->dev.platform_data = &priv->ssdd_dt;
-
 	return 0;
 }
 
@@ -1091,7 +1035,6 @@ static int ov2640_probe(struct i2c_client *client,
 			const struct i2c_device_id *did)
 {
 	struct ov2640_priv	*priv;
-	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
 	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
 	int			ret;
 
@@ -1112,17 +1055,15 @@ static int ov2640_probe(struct i2c_client *client,
 	if (IS_ERR(priv->clk))
 		return -EPROBE_DEFER;
 
-	if (!ssdd && !client->dev.of_node) {
+	if (!client->dev.of_node) {
 		dev_err(&client->dev, "Missing platform_data for driver\n");
 		ret = -EINVAL;
 		goto err_clk;
 	}
 
-	if (!ssdd) {
-		ret = ov2640_probe_dt(client, priv);
-		if (ret)
-			goto err_clk;
-	}
+	ret = ov2640_probe_dt(client, priv);
+	if (ret)
+		goto err_clk;
 
 	v4l2_i2c_subdev_init(&priv->subdev, client, &ov2640_subdev_ops);
 	v4l2_ctrl_handler_init(&priv->hdl, 2);
@@ -1190,6 +1131,6 @@ static struct i2c_driver ov2640_i2c_driver = {
 
 module_i2c_driver(ov2640_i2c_driver);
 
-MODULE_DESCRIPTION("SoC Camera driver for Omni Vision 2640 sensor");
+MODULE_DESCRIPTION("Driver for Omni Vision 2640 sensor");
 MODULE_AUTHOR("Alberto Panizzo");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/media/i2c/soc_camera/Kconfig b/drivers/media/i2c/soc_camera/Kconfig
index 7704bcf5cc25..96859f37cb1c 100644
--- a/drivers/media/i2c/soc_camera/Kconfig
+++ b/drivers/media/i2c/soc_camera/Kconfig
@@ -41,12 +41,6 @@ config SOC_CAMERA_MT9V022
 	help
 	  This driver supports MT9V022 cameras from Micron
 
-config SOC_CAMERA_OV2640
-	tristate "ov2640 camera support"
-	depends on SOC_CAMERA && I2C
-	help
-	  This is a ov2640 camera driver
-
 config SOC_CAMERA_OV5642
 	tristate "ov5642 camera support"
 	depends on SOC_CAMERA && I2C
diff --git a/drivers/media/i2c/soc_camera/Makefile b/drivers/media/i2c/soc_camera/Makefile
index 6f994f9353a0..974bdb721dbe 100644
--- a/drivers/media/i2c/soc_camera/Makefile
+++ b/drivers/media/i2c/soc_camera/Makefile
@@ -3,7 +3,6 @@ obj-$(CONFIG_SOC_CAMERA_MT9M001)	+= mt9m001.o
 obj-$(CONFIG_SOC_CAMERA_MT9T031)	+= mt9t031.o
 obj-$(CONFIG_SOC_CAMERA_MT9T112)	+= mt9t112.o
 obj-$(CONFIG_SOC_CAMERA_MT9V022)	+= mt9v022.o
-obj-$(CONFIG_SOC_CAMERA_OV2640)		+= ov2640.o
 obj-$(CONFIG_SOC_CAMERA_OV5642)		+= ov5642.o
 obj-$(CONFIG_SOC_CAMERA_OV6650)		+= ov6650.o
 obj-$(CONFIG_SOC_CAMERA_OV772X)		+= ov772x.o
-- 
2.11.0

[PATCHv3 11/15] ov2640: use standard clk and enable it.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

Convert v4l2_clk to normal clk and enable the clock.

Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
 drivers/media/i2c/ov2640.c | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index 83f88efbce69..2f799ab4cc2b 100644
--- a/drivers/media/i2c/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -16,6 +16,7 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/i2c.h>
+#include <linux/clk.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/gpio.h>
@@ -24,7 +25,6 @@
 #include <linux/v4l2-mediabus.h>
 #include <linux/videodev2.h>
 
-#include <media/v4l2-clk.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-subdev.h>
 #include <media/v4l2-ctrls.h>
@@ -284,7 +284,7 @@ struct ov2640_priv {
 	struct v4l2_subdev		subdev;
 	struct v4l2_ctrl_handler	hdl;
 	u32	cfmt_code;
-	struct v4l2_clk			*clk;
+	struct clk			*clk;
 	const struct ov2640_win_size	*win;
 
 	struct gpio_desc *resetb_gpio;
@@ -1051,19 +1051,16 @@ static int ov2640_probe(struct i2c_client *client,
 		return -ENOMEM;
 	}
 
-	priv->clk = v4l2_clk_get(&client->dev, "xvclk");
-	if (IS_ERR(priv->clk))
-		return -EPROBE_DEFER;
-
-	if (!client->dev.of_node) {
-		dev_err(&client->dev, "Missing platform_data for driver\n");
-		ret = -EINVAL;
-		goto err_clk;
+	if (client->dev.of_node) {
+		priv->clk = devm_clk_get(&client->dev, "xvclk");
+		if (IS_ERR(priv->clk))
+			return -EPROBE_DEFER;
+		clk_prepare_enable(priv->clk);
 	}
 
 	ret = ov2640_probe_dt(client, priv);
 	if (ret)
-		goto err_clk;
+		return ret;
 
 	v4l2_i2c_subdev_init(&priv->subdev, client, &ov2640_subdev_ops);
 	v4l2_ctrl_handler_init(&priv->hdl, 2);
@@ -1074,25 +1071,23 @@ static int ov2640_probe(struct i2c_client *client,
 	priv->subdev.ctrl_handler = &priv->hdl;
 	if (priv->hdl.error) {
 		ret = priv->hdl.error;
-		goto err_clk;
+		goto err_hdl;
 	}
 
 	ret = ov2640_video_probe(client);
 	if (ret < 0)
-		goto err_videoprobe;
+		goto err_hdl;
 
 	ret = v4l2_async_register_subdev(&priv->subdev);
 	if (ret < 0)
-		goto err_videoprobe;
+		goto err_hdl;
 
 	dev_info(&adapter->dev, "OV2640 Probed\n");
 
 	return 0;
 
-err_videoprobe:
+err_hdl:
 	v4l2_ctrl_handler_free(&priv->hdl);
-err_clk:
-	v4l2_clk_put(priv->clk);
 	return ret;
 }
 
@@ -1101,9 +1096,8 @@ static int ov2640_remove(struct i2c_client *client)
 	struct ov2640_priv       *priv = to_ov2640(client);
 
 	v4l2_async_unregister_subdev(&priv->subdev);
-	v4l2_clk_put(priv->clk);
-	v4l2_device_unregister_subdev(&priv->subdev);
 	v4l2_ctrl_handler_free(&priv->hdl);
+	v4l2_device_unregister_subdev(&priv->subdev);
 	return 0;
 }
 
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 12/15] ov2640: add MC support

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

The MC support is needed by the em28xx driver.

Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
 drivers/media/i2c/ov2640.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index 2f799ab4cc2b..3c187069ec26 100644
--- a/drivers/media/i2c/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -282,6 +282,9 @@ struct ov2640_win_size {
 
 struct ov2640_priv {
 	struct v4l2_subdev		subdev;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	struct media_pad pad;
+#endif
 	struct v4l2_ctrl_handler	hdl;
 	u32	cfmt_code;
 	struct clk			*clk;
@@ -1073,19 +1076,30 @@ static int ov2640_probe(struct i2c_client *client,
 		ret = priv->hdl.error;
 		goto err_hdl;
 	}
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	priv->pad.flags = MEDIA_PAD_FL_SOURCE;
+	priv->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+	ret = media_entity_pads_init(&priv->subdev.entity, 1, &priv->pad);
+	if (ret < 0)
+		goto err_hdl;
+#endif
 
 	ret = ov2640_video_probe(client);
 	if (ret < 0)
-		goto err_hdl;
+		goto err_videoprobe;
 
 	ret = v4l2_async_register_subdev(&priv->subdev);
 	if (ret < 0)
-		goto err_hdl;
+		goto err_videoprobe;
 
 	dev_info(&adapter->dev, "OV2640 Probed\n");
 
 	return 0;
 
+err_videoprobe:
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	media_entity_cleanup(&priv->subdev.entity);
+#endif
 err_hdl:
 	v4l2_ctrl_handler_free(&priv->hdl);
 	return ret;
@@ -1097,6 +1111,9 @@ static int ov2640_remove(struct i2c_client *client)
 
 	v4l2_async_unregister_subdev(&priv->subdev);
 	v4l2_ctrl_handler_free(&priv->hdl);
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	media_entity_cleanup(&priv->subdev.entity);
+#endif
 	v4l2_device_unregister_subdev(&priv->subdev);
 	return 0;
 }
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 13/15] em28xx: drop last soc_camera link

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

The em28xx driver still used the soc_camera.h header for the ov2640
driver. Since this driver no longer uses soc_camera, that include can
be removed.

Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
 drivers/media/usb/em28xx/em28xx-camera.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c
index 89c890ba7dd6..63aaa577a742 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -23,7 +23,6 @@
 
 #include <linux/i2c.h>
 #include <linux/usb.h>
-#include <media/soc_camera.h>
 #include <media/i2c/mt9v011.h>
 #include <media/v4l2-clk.h>
 #include <media/v4l2-common.h>
@@ -43,13 +42,6 @@ static unsigned short omnivision_sensor_addrs[] = {
 	I2C_CLIENT_END
 };
 
-static struct soc_camera_link camlink = {
-	.bus_id = 0,
-	.flags = 0,
-	.module_name = "em28xx",
-	.unbalanced_power = true,
-};
-
 /* FIXME: Should be replaced by a proper mt9m111 driver */
 static int em28xx_initialize_mt9m111(struct em28xx *dev)
 {
@@ -421,7 +413,6 @@ int em28xx_init_camera(struct em28xx *dev)
 			.type = "ov2640",
 			.flags = I2C_CLIENT_SCCB,
 			.addr = client->addr,
-			.platform_data = &camlink,
 		};
 		struct v4l2_subdev_format format = {
 			.which = V4L2_SUBDEV_FORMAT_ACTIVE,
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 14/15] sama5d3 dts: enable atmel-isi

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This illustrates the changes needed to the dts in order to hook up the
ov7670. I don't plan on merging this.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 arch/arm/boot/dts/at91-sama5d3_xplained.dts | 60 ++++++++++++++++++++++++++---
 arch/arm/boot/dts/sama5d3.dtsi              |  4 +-
 2 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/at91-sama5d3_xplained.dts b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
index c51fc652f6c7..b10f8b9e6375 100644
--- a/arch/arm/boot/dts/at91-sama5d3_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
@@ -65,18 +65,52 @@
 				status = "okay";
 			};
 
+			isi0: isi@f0034000 {
+				status = "okay";
+				port {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					isi_0: endpoint {
+						remote-endpoint = <&ov7670_0>;
+						bus-width = <8>;
+						vsync-active = <1>;
+						hsync-active = <1>;
+					};
+				};
+			};
+
 			i2c0: i2c@f0014000 {
 				pinctrl-0 = <&pinctrl_i2c0_pu>;
-				status = "okay";
+				status = "disabled";
 			};
 
 			i2c1: i2c@f0018000 {
 				status = "okay";
 
+				ov7670: camera@21 {
+					compatible = "ovti,ov7670";
+					reg = <0x21>;
+					pinctrl-names = "default";
+					pinctrl-0 = <&pinctrl_pck0_as_isi_mck &pinctrl_sensor_power &pinctrl_sensor_reset>;
+					reset-gpios = <&pioE 11 GPIO_ACTIVE_LOW>;
+					powerdown-gpios = <&pioE 13 GPIO_ACTIVE_HIGH>;
+					clocks = <&pck0>;
+					clock-names = "xclk";
+					assigned-clocks = <&pck0>;
+					assigned-clock-rates = <25000000>;
+
+					port {
+						ov7670_0: endpoint {
+							remote-endpoint = <&isi_0>;
+							bus-width = <8>;
+						};
+					};
+				};
+
 				pmic: act8865@5b {
 					compatible = "active-semi,act8865";
 					reg = <0x5b>;
-					status = "disabled";
+					status = "okay";
 
 					regulators {
 						vcc_1v8_reg: DCDC_REG1 {
@@ -130,7 +164,7 @@
 			pwm0: pwm@f002c000 {
 				pinctrl-names = "default";
 				pinctrl-0 = <&pinctrl_pwm0_pwmh0_0 &pinctrl_pwm0_pwmh1_0>;
-				status = "okay";
+				status = "disabled";
 			};
 
 			usart0: serial@f001c000 {
@@ -143,7 +177,7 @@
 			};
 
 			uart0: serial@f0024000 {
-				status = "okay";
+				status = "disabled";
 			};
 
 			mmc1: mmc@f8000000 {
@@ -181,7 +215,7 @@
 			i2c2: i2c@f801c000 {
 				dmas = <0>, <0>;	/* Do not use DMA for i2c2 */
 				pinctrl-0 = <&pinctrl_i2c2_pu>;
-				status = "okay";
+				status = "disabled";
 			};
 
 			macb1: ethernet@f802c000 {
@@ -200,6 +234,22 @@
 			};
 
 			pinctrl@fffff200 {
+				camera_sensor {
+					pinctrl_pck0_as_isi_mck: pck0_as_isi_mck-0 {
+						atmel,pins =
+							<AT91_PIOD 30 AT91_PERIPH_B AT91_PINCTRL_NONE>;	/* ISI_MCK */
+					};
+
+					pinctrl_sensor_power: sensor_power-0 {
+						atmel,pins =
+							<AT91_PIOE 13 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
+					};
+
+					pinctrl_sensor_reset: sensor_reset-0 {
+						atmel,pins =
+							<AT91_PIOE 11 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
+					};
+				};
 				board {
 					pinctrl_i2c0_pu: i2c0_pu {
 						atmel,pins =
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index b06448ba6649..099570e4b90a 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -176,7 +176,7 @@
 				#address-cells = <1>;
 				#size-cells = <0>;
 				clocks = <&twi1_clk>;
-				status = "disabled";
+				status = "ok";
 			};
 
 			usart0: serial@f001c000 {
@@ -235,7 +235,7 @@
 				pinctrl-0 = <&pinctrl_isi_data_0_7>;
 				clocks = <&isi_clk>;
 				clock-names = "isi_clk";
-				status = "disabled";
+				status = "ok";
 				port {
 					#address-cells = <1>;
 					#size-cells = <0>;
-- 
2.11.0

[PATCHv3 15/15] at91-sama5d3_xplained.dts: select ov2640

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This patch replaces the ov7670 with the ov2640. This patch is not
meant to be merged but is for demonstration purposes only.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 arch/arm/boot/dts/at91-sama5d3_xplained.dts | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/at91-sama5d3_xplained.dts b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
index b10f8b9e6375..65ccb35213a9 100644
--- a/arch/arm/boot/dts/at91-sama5d3_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
@@ -71,7 +71,7 @@
 					#address-cells = <1>;
 					#size-cells = <0>;
 					isi_0: endpoint {
-						remote-endpoint = <&ov7670_0>;
+						remote-endpoint = <&ov2640_0>;
 						bus-width = <8>;
 						vsync-active = <1>;
 						hsync-active = <1>;
@@ -87,20 +87,20 @@
 			i2c1: i2c@f0018000 {
 				status = "okay";
 
-				ov7670: camera@21 {
-					compatible = "ovti,ov7670";
-					reg = <0x21>;
+				ov2640: camera@30 {
+					compatible = "ovti,ov2640";
+					reg = <0x30>;
 					pinctrl-names = "default";
 					pinctrl-0 = <&pinctrl_pck0_as_isi_mck &pinctrl_sensor_power &pinctrl_sensor_reset>;
-					reset-gpios = <&pioE 11 GPIO_ACTIVE_LOW>;
-					powerdown-gpios = <&pioE 13 GPIO_ACTIVE_HIGH>;
+					resetb-gpios = <&pioE 11 GPIO_ACTIVE_LOW>;
+					pwdn-gpios = <&pioE 13 GPIO_ACTIVE_HIGH>;
 					clocks = <&pck0>;
-					clock-names = "xclk";
+					clock-names = "xvclk";
 					assigned-clocks = <&pck0>;
 					assigned-clock-rates = <25000000>;
 
 					port {
-						ov7670_0: endpoint {
+						ov2640_0: endpoint {
 							remote-endpoint = <&isi_0>;
 							bus-width = <8>;
 						};
-- 
2.11.0

Re: [PATCHv3 10/15] ov2640: convert from soc-camera to a standard subdev sensor driver.

[Frank Schäfer]

Am 06.03.2017 um 15:56 schrieb Hans Verkuil:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Convert ov2640 to a standard subdev driver. The soc-camera driver no longer
> uses this driver, so it can safely be converted.
>
> Note: the s_power op has been dropped: this never worked. When the last open()
> is closed, then the power is turned off, and when it is opened again the power
> is turned on again, but the old state isn't restored.
>
> Someone else can figure out in the future how to get this working correctly,
> but I don't want to spend more time on this.
Last two sections of this description are a leftover from v2. ;)

Regards,
Frank

>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>   drivers/media/i2c/Kconfig                   | 11 ++++
>   drivers/media/i2c/Makefile                  |  1 +
>   drivers/media/i2c/{soc_camera => }/ov2640.c | 89 +++++------------------------
>   drivers/media/i2c/soc_camera/Kconfig        |  6 --
>   drivers/media/i2c/soc_camera/Makefile       |  1 -
>   5 files changed, 27 insertions(+), 81 deletions(-)
>   rename drivers/media/i2c/{soc_camera => }/ov2640.c (94%)
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index cee1dae6e014..db2c63f592c5 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -520,6 +520,17 @@ config VIDEO_APTINA_PLL
>   config VIDEO_SMIAPP_PLL
>   	tristate
>   
> +config VIDEO_OV2640
> +	tristate "OmniVision OV2640 sensor support"
> +	depends on VIDEO_V4L2 && I2C && GPIOLIB
> +	depends on MEDIA_CAMERA_SUPPORT
> +	help
> +	  This is a Video4Linux2 sensor-level driver for the OmniVision
> +	  OV2640 camera.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ov2640.
> +
>   config VIDEO_OV2659
>   	tristate "OmniVision OV2659 sensor support"
>   	depends on VIDEO_V4L2 && I2C
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 5bc7bbeb5499..50af1e11c85a 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -57,6 +57,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
>   obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
>   obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
>   obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
> +obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
>   obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
>   obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
>   obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
> diff --git a/drivers/media/i2c/soc_camera/ov2640.c b/drivers/media/i2c/ov2640.c
> similarity index 94%
> rename from drivers/media/i2c/soc_camera/ov2640.c
> rename to drivers/media/i2c/ov2640.c
> index b9a0069f5b33..83f88efbce69 100644
> --- a/drivers/media/i2c/soc_camera/ov2640.c
> +++ b/drivers/media/i2c/ov2640.c
> @@ -24,8 +24,8 @@
>   #include <linux/v4l2-mediabus.h>
>   #include <linux/videodev2.h>
>   
> -#include <media/soc_camera.h>
>   #include <media/v4l2-clk.h>
> +#include <media/v4l2-device.h>
>   #include <media/v4l2-subdev.h>
>   #include <media/v4l2-ctrls.h>
>   #include <media/v4l2-image-sizes.h>
> @@ -287,7 +287,6 @@ struct ov2640_priv {
>   	struct v4l2_clk			*clk;
>   	const struct ov2640_win_size	*win;
>   
> -	struct soc_camera_subdev_desc	ssdd_dt;
>   	struct gpio_desc *resetb_gpio;
>   	struct gpio_desc *pwdn_gpio;
>   };
> @@ -677,13 +676,8 @@ static int ov2640_reset(struct i2c_client *client)
>   }
>   
>   /*
> - * soc_camera_ops functions
> + * functions
>    */
> -static int ov2640_s_stream(struct v4l2_subdev *sd, int enable)
> -{
> -	return 0;
> -}
> -
>   static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
>   {
>   	struct v4l2_subdev *sd =
> @@ -744,10 +738,16 @@ static int ov2640_s_register(struct v4l2_subdev *sd,
>   static int ov2640_s_power(struct v4l2_subdev *sd, int on)
>   {
>   	struct i2c_client *client = v4l2_get_subdevdata(sd);
> -	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
>   	struct ov2640_priv *priv = to_ov2640(client);
>   
> -	return soc_camera_set_power(&client->dev, ssdd, priv->clk, on);
> +	gpiod_direction_output(priv->pwdn_gpio, !on);
> +	if (on && priv->resetb_gpio) {
> +		/* Active the resetb pin to perform a reset pulse */
> +		gpiod_direction_output(priv->resetb_gpio, 1);
> +		usleep_range(3000, 5000);
> +		gpiod_direction_output(priv->resetb_gpio, 0);
> +	}
> +	return 0;
>   }
>   
>   /* Select the nearest higher resolution for capture */
> @@ -994,26 +994,6 @@ static struct v4l2_subdev_core_ops ov2640_subdev_core_ops = {
>   	.s_power	= ov2640_s_power,
>   };
>   
> -static int ov2640_g_mbus_config(struct v4l2_subdev *sd,
> -				struct v4l2_mbus_config *cfg)
> -{
> -	struct i2c_client *client = v4l2_get_subdevdata(sd);
> -	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> -
> -	cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
> -		V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> -		V4L2_MBUS_DATA_ACTIVE_HIGH;
> -	cfg->type = V4L2_MBUS_PARALLEL;
> -	cfg->flags = soc_camera_apply_board_flags(ssdd, cfg);
> -
> -	return 0;
> -}
> -
> -static struct v4l2_subdev_video_ops ov2640_subdev_video_ops = {
> -	.s_stream	= ov2640_s_stream,
> -	.g_mbus_config	= ov2640_g_mbus_config,
> -};
> -
>   static const struct v4l2_subdev_pad_ops ov2640_subdev_pad_ops = {
>   	.enum_mbus_code = ov2640_enum_mbus_code,
>   	.get_selection	= ov2640_get_selection,
> @@ -1023,40 +1003,9 @@ static const struct v4l2_subdev_pad_ops ov2640_subdev_pad_ops = {
>   
>   static struct v4l2_subdev_ops ov2640_subdev_ops = {
>   	.core	= &ov2640_subdev_core_ops,
> -	.video	= &ov2640_subdev_video_ops,
>   	.pad	= &ov2640_subdev_pad_ops,
>   };
>   
> -/* OF probe functions */
> -static int ov2640_hw_power(struct device *dev, int on)
> -{
> -	struct i2c_client *client = to_i2c_client(dev);
> -	struct ov2640_priv *priv = to_ov2640(client);
> -
> -	dev_dbg(&client->dev, "%s: %s the camera\n",
> -			__func__, on ? "ENABLE" : "DISABLE");
> -
> -	if (priv->pwdn_gpio)
> -		gpiod_direction_output(priv->pwdn_gpio, !on);
> -
> -	return 0;
> -}
> -
> -static int ov2640_hw_reset(struct device *dev)
> -{
> -	struct i2c_client *client = to_i2c_client(dev);
> -	struct ov2640_priv *priv = to_ov2640(client);
> -
> -	if (priv->resetb_gpio) {
> -		/* Active the resetb pin to perform a reset pulse */
> -		gpiod_direction_output(priv->resetb_gpio, 1);
> -		usleep_range(3000, 5000);
> -		gpiod_direction_output(priv->resetb_gpio, 0);
> -	}
> -
> -	return 0;
> -}
> -
>   static int ov2640_probe_dt(struct i2c_client *client,
>   		struct ov2640_priv *priv)
>   {
> @@ -1076,11 +1025,6 @@ static int ov2640_probe_dt(struct i2c_client *client,
>   	else if (IS_ERR(priv->pwdn_gpio))
>   		return PTR_ERR(priv->pwdn_gpio);
>   
> -	/* Initialize the soc_camera_subdev_desc */
> -	priv->ssdd_dt.power = ov2640_hw_power;
> -	priv->ssdd_dt.reset = ov2640_hw_reset;
> -	client->dev.platform_data = &priv->ssdd_dt;
> -
>   	return 0;
>   }
>   
> @@ -1091,7 +1035,6 @@ static int ov2640_probe(struct i2c_client *client,
>   			const struct i2c_device_id *did)
>   {
>   	struct ov2640_priv	*priv;
> -	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
>   	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
>   	int			ret;
>   
> @@ -1112,17 +1055,15 @@ static int ov2640_probe(struct i2c_client *client,
>   	if (IS_ERR(priv->clk))
>   		return -EPROBE_DEFER;
>   
> -	if (!ssdd && !client->dev.of_node) {
> +	if (!client->dev.of_node) {
>   		dev_err(&client->dev, "Missing platform_data for driver\n");
>   		ret = -EINVAL;
>   		goto err_clk;
>   	}
>   
> -	if (!ssdd) {
> -		ret = ov2640_probe_dt(client, priv);
> -		if (ret)
> -			goto err_clk;
> -	}
> +	ret = ov2640_probe_dt(client, priv);
> +	if (ret)
> +		goto err_clk;
>   
>   	v4l2_i2c_subdev_init(&priv->subdev, client, &ov2640_subdev_ops);
>   	v4l2_ctrl_handler_init(&priv->hdl, 2);
> @@ -1190,6 +1131,6 @@ static struct i2c_driver ov2640_i2c_driver = {
>   
>   module_i2c_driver(ov2640_i2c_driver);
>   
> -MODULE_DESCRIPTION("SoC Camera driver for Omni Vision 2640 sensor");
> +MODULE_DESCRIPTION("Driver for Omni Vision 2640 sensor");
>   MODULE_AUTHOR("Alberto Panizzo");
>   MODULE_LICENSE("GPL v2");
> diff --git a/drivers/media/i2c/soc_camera/Kconfig b/drivers/media/i2c/soc_camera/Kconfig
> index 7704bcf5cc25..96859f37cb1c 100644
> --- a/drivers/media/i2c/soc_camera/Kconfig
> +++ b/drivers/media/i2c/soc_camera/Kconfig
> @@ -41,12 +41,6 @@ config SOC_CAMERA_MT9V022
>   	help
>   	  This driver supports MT9V022 cameras from Micron
>   
> -config SOC_CAMERA_OV2640
> -	tristate "ov2640 camera support"
> -	depends on SOC_CAMERA && I2C
> -	help
> -	  This is a ov2640 camera driver
> -
>   config SOC_CAMERA_OV5642
>   	tristate "ov5642 camera support"
>   	depends on SOC_CAMERA && I2C
> diff --git a/drivers/media/i2c/soc_camera/Makefile b/drivers/media/i2c/soc_camera/Makefile
> index 6f994f9353a0..974bdb721dbe 100644
> --- a/drivers/media/i2c/soc_camera/Makefile
> +++ b/drivers/media/i2c/soc_camera/Makefile
> @@ -3,7 +3,6 @@ obj-$(CONFIG_SOC_CAMERA_MT9M001)	+= mt9m001.o
>   obj-$(CONFIG_SOC_CAMERA_MT9T031)	+= mt9t031.o
>   obj-$(CONFIG_SOC_CAMERA_MT9T112)	+= mt9t112.o
>   obj-$(CONFIG_SOC_CAMERA_MT9V022)	+= mt9v022.o
> -obj-$(CONFIG_SOC_CAMERA_OV2640)		+= ov2640.o
>   obj-$(CONFIG_SOC_CAMERA_OV5642)		+= ov5642.o
>   obj-$(CONFIG_SOC_CAMERA_OV6650)		+= ov6650.o
>   obj-$(CONFIG_SOC_CAMERA_OV772X)		+= ov772x.o

Re: [PATCHv3 01/15] ov7670: document device tree bindings

[Sakari Ailus]

Hi Hans,

On Mon, Mar 06, 2017 at 03:56:02PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> 
> Add binding documentation and add that file to the MAINTAINERS entry.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/media/i2c/ov7670.txt       | 44 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7670.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov7670.txt b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
> new file mode 100644
> index 000000000000..6d9c90dff7a7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
> @@ -0,0 +1,44 @@
> +* Omnivision OV7670 CMOS sensor
> +
> +The Omnivision OV7670 sensor supports multiple resolutions output, such as
> +CIF, SVGA, UXGA. It also can support the YUV422/420, RGB565/555 or raw RGB
> +output formats.
> +
> +Required Properties:
> +- compatible: should be "ovti,ov7670"
> +- clocks: reference to the xclk input clock.
> +- clock-names: should be "xclk".
> +
> +Optional Properties:
> +- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
> +  Active is low.
> +- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
> +  Active is high.
> +
> +The device node must contain one 'port' child node for its digital output
> +video port, in accordance with the video interface bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +Example:
> +
> +	i2c1: i2c@f0018000 {
> +		ov7670: camera@21 {
> +			compatible = "ovti,ov7670";
> +			reg = <0x21>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_pck0_as_isi_mck &pinctrl_sensor_power &pinctrl_sensor_reset>;
> +			reset-gpios = <&pioE 11 GPIO_ACTIVE_LOW>;
> +			powerdown-gpios = <&pioE 13 GPIO_ACTIVE_HIGH>;
> +			clocks = <&pck0>;
> +			clock-names = "xclk";
> +			assigned-clocks = <&pck0>;
> +			assigned-clock-rates = <25000000>;
> +
> +			port {
> +				ov7670_0: endpoint {
> +					remote-endpoint = <&isi_0>;
> +					bus-width = <8>;

Didn't I previously request to specify which of the standardised properties
are relevant for the device (and which ones are required and which are
optional)? If I didn't, I'm doing that now. :-)

E.g. the omap3isp driver documentation looks like this:

Documentation/devicetree/bindings/media/ti,omap3isp.txt

> +				};
> +			};
> +		};
> +	};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 83a42ef1d1a7..93500928ca4f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9273,6 +9273,7 @@ L:	linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>  T:	git git://linuxtv.org/media_tree.git
>  S:	Maintained
>  F:	drivers/media/i2c/ov7670.c
> +F:	Documentation/devicetree/bindings/media/i2c/ov7670.txt
>  
>  ONENAND FLASH DRIVER
>  M:	Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 03/15] ov7670: fix g/s_parm

[Sakari Ailus]

Hi Hans,

On Mon, Mar 06, 2017 at 03:56:04PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> 
> Drop unnecesary memset. Drop the unnecessary extendedmode check and
> set the V4L2_CAP_TIMEPERFRAME capability.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/media/i2c/ov7670.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> index 9af8d3b8f848..50e4466a2b37 100644
> --- a/drivers/media/i2c/ov7670.c
> +++ b/drivers/media/i2c/ov7670.c
> @@ -1046,7 +1046,6 @@ static int ov7670_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
>  	if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		return -EINVAL;
>  
> -	memset(cp, 0, sizeof(struct v4l2_captureparm));
>  	cp->capability = V4L2_CAP_TIMEPERFRAME;
>  	info->devtype->get_framerate(sd, &cp->timeperframe);
>  
> @@ -1061,9 +1060,8 @@ static int ov7670_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
>  
>  	if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		return -EINVAL;
> -	if (cp->extendedmode != 0)
> -		return -EINVAL;
>  
> +	cp->capability = V4L2_CAP_TIMEPERFRAME;
>  	return info->devtype->set_framerate(sd, tpf);
>  }
>  

We seem to have two ways to specify the frame interval for sub-devices,
with roughly roughly similar number of users. s_parm() originates from
drivers that typically work on plain V4L2 interfaces whereas
s_frame_interval() is from native sub-device drivers.

Anyway,

Acked-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

-- 
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 04/15] ov7670: get xclk

[Sakari Ailus]

Hi Hans,

On Mon, Mar 06, 2017 at 03:56:05PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Get the clock for this sensor.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/i2c/ov7670.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> index 50e4466a2b37..da0843617a49 100644
> --- a/drivers/media/i2c/ov7670.c
> +++ b/drivers/media/i2c/ov7670.c
> @@ -10,6 +10,7 @@
>   * This file may be distributed under the terms of the GNU General
>   * Public License, version 2.
>   */
> +#include <linux/clk.h>
>  #include <linux/init.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> @@ -227,6 +228,7 @@ struct ov7670_info {
>  		struct v4l2_ctrl *hue;
>  	};
>  	struct ov7670_format_struct *fmt;  /* Current format */
> +	struct clk *clk;
>  	int min_width;			/* Filter out smaller sizes */
>  	int min_height;			/* Filter out smaller sizes */
>  	int clock_speed;		/* External clock speed (MHz) */
> @@ -1587,6 +1589,15 @@ static int ov7670_probe(struct i2c_client *client,
>  			info->pclk_hb_disable = true;
>  	}
>  
> +	info->clk = devm_clk_get(&client->dev, "xclk");
> +	if (IS_ERR(info->clk))
> +		return -EPROBE_DEFER;
> +	clk_prepare_enable(info->clk);
> +
> +	info->clock_speed = clk_get_rate(info->clk) / 1000000;
> +	if (info->clock_speed < 10 || info->clock_speed > 48)
> +		return -EINVAL;

clk_disable_unprepare() before return?

> +
>  	/* Make sure it's an ov7670 */
>  	ret = ov7670_detect(sd);
>  	if (ret) {
> @@ -1667,6 +1678,7 @@ static int ov7670_remove(struct i2c_client *client)
>  
>  	v4l2_device_unregister_subdev(sd);
>  	v4l2_ctrl_handler_free(&info->hdl);
> +	clk_disable_unprepare(info->clk);
>  	return 0;
>  }
>  

-- 
Kind regards,

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

Re: [PATCHv3 05/15] ov7670: add devicetree support

[Sakari Ailus]

Hi Hans,

On Mon, Mar 06, 2017 at 03:56:06PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> 
> Add DT support. Use it to get the reset and pwdn pins (if there are any).
> Tested with one sensor requiring reset/pwdn and one sensor that doesn't
> have reset/pwdn pins.

If I read the datasheet right, lifting the reset signal up will reset the
sensor but the patch doesn't make use of that, it only ensures the reset
signal stays low. Should you lift it up for a while as well? The datasheet
doesn't say for how long that should be done, but that it should be usable
after 1 ms since pulling reset down.

> 
> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/media/i2c/ov7670.c | 40 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> index da0843617a49..f9d977430dcf 100644
> --- a/drivers/media/i2c/ov7670.c
> +++ b/drivers/media/i2c/ov7670.c
> @@ -17,6 +17,8 @@
>  #include <linux/i2c.h>
>  #include <linux/delay.h>
>  #include <linux/videodev2.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-mediabus.h>
> @@ -229,6 +231,8 @@ struct ov7670_info {
>  	};
>  	struct ov7670_format_struct *fmt;  /* Current format */
>  	struct clk *clk;
> +	struct gpio_desc *resetb_gpio;
> +	struct gpio_desc *pwdn_gpio;
>  	int min_width;			/* Filter out smaller sizes */
>  	int min_height;			/* Filter out smaller sizes */
>  	int clock_speed;		/* External clock speed (MHz) */
> @@ -591,8 +595,6 @@ static int ov7670_init(struct v4l2_subdev *sd, u32 val)
>  	return ov7670_write_array(sd, ov7670_default_regs);
>  }
>  
> -
> -
>  static int ov7670_detect(struct v4l2_subdev *sd)
>  {
>  	unsigned char v;
> @@ -1549,6 +1551,27 @@ static const struct ov7670_devtype ov7670_devdata[] = {
>  	},
>  };
>  
> +static int ov7670_init_gpio(struct i2c_client *client, struct ov7670_info *info)
> +{
> +	info->pwdn_gpio = devm_gpiod_get_optional(&client->dev, "powerdown",
> +			GPIOD_OUT_LOW);
> +	if (IS_ERR(info->pwdn_gpio)) {
> +		dev_info(&client->dev, "can't get %s GPIO\n", "powerdown");
> +		return PTR_ERR(info->pwdn_gpio);
> +	}
> +
> +	info->resetb_gpio = devm_gpiod_get_optional(&client->dev, "reset",
> +			GPIOD_OUT_LOW);
> +	if (IS_ERR(info->resetb_gpio)) {
> +		dev_info(&client->dev, "can't get %s GPIO\n", "reset");
> +		return PTR_ERR(info->resetb_gpio);
> +	}
> +
> +	usleep_range(3000, 5000);
> +
> +	return 0;
> +}
> +
>  static int ov7670_probe(struct i2c_client *client,
>  			const struct i2c_device_id *id)
>  {
> @@ -1594,6 +1617,10 @@ static int ov7670_probe(struct i2c_client *client,
>  		return -EPROBE_DEFER;
>  	clk_prepare_enable(info->clk);
>  
> +	ret = ov7670_init_gpio(client, info);
> +	if (ret)
> +		return ret;
> +
>  	info->clock_speed = clk_get_rate(info->clk) / 1000000;
>  	if (info->clock_speed < 10 || info->clock_speed > 48)
>  		return -EINVAL;
> @@ -1689,9 +1716,18 @@ static const struct i2c_device_id ov7670_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, ov7670_id);
>  
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id ov7670_of_match[] = {
> +	{ .compatible = "ovti,ov7670", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, ov7670_of_match);
> +#endif
> +
>  static struct i2c_driver ov7670_driver = {
>  	.driver = {
>  		.name	= "ov7670",
> +		.of_match_table = of_match_ptr(ov7670_of_match),
>  	},
>  	.probe		= ov7670_probe,
>  	.remove		= ov7670_remove,

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 04/15] ov7670: get xclk

[Hans Verkuil]

On 09/03/17 21:38, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, Mar 06, 2017 at 03:56:05PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
>>
>> Get the clock for this sensor.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
>> ---
>>  drivers/media/i2c/ov7670.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
>> index 50e4466a2b37..da0843617a49 100644
>> --- a/drivers/media/i2c/ov7670.c
>> +++ b/drivers/media/i2c/ov7670.c
>> @@ -10,6 +10,7 @@
>>   * This file may be distributed under the terms of the GNU General
>>   * Public License, version 2.
>>   */
>> +#include <linux/clk.h>
>>  #include <linux/init.h>
>>  #include <linux/module.h>
>>  #include <linux/slab.h>
>> @@ -227,6 +228,7 @@ struct ov7670_info {
>>  		struct v4l2_ctrl *hue;
>>  	};
>>  	struct ov7670_format_struct *fmt;  /* Current format */
>> +	struct clk *clk;
>>  	int min_width;			/* Filter out smaller sizes */
>>  	int min_height;			/* Filter out smaller sizes */
>>  	int clock_speed;		/* External clock speed (MHz) */
>> @@ -1587,6 +1589,15 @@ static int ov7670_probe(struct i2c_client *client,
>>  			info->pclk_hb_disable = true;
>>  	}
>>  
>> +	info->clk = devm_clk_get(&client->dev, "xclk");
>> +	if (IS_ERR(info->clk))
>> +		return -EPROBE_DEFER;
>> +	clk_prepare_enable(info->clk);
>> +
>> +	info->clock_speed = clk_get_rate(info->clk) / 1000000;
>> +	if (info->clock_speed < 10 || info->clock_speed > 48)
>> +		return -EINVAL;
> 
> clk_disable_unprepare() before return?

It is my understanding that devm_clk_get will call that for you.

Correct me if I am wrong.

Regards,

	Hans

> 
>> +
>>  	/* Make sure it's an ov7670 */
>>  	ret = ov7670_detect(sd);
>>  	if (ret) {
>> @@ -1667,6 +1678,7 @@ static int ov7670_remove(struct i2c_client *client)
>>  
>>  	v4l2_device_unregister_subdev(sd);
>>  	v4l2_ctrl_handler_free(&info->hdl);
>> +	clk_disable_unprepare(info->clk);
>>  	return 0;
>>  }
>>  
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 04/15] ov7670: get xclk

[Sakari Ailus]

Hi Hans,

On Fri, Mar 10, 2017 at 09:55:53AM +0100, Hans Verkuil wrote:
> On 09/03/17 21:38, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > On Mon, Mar 06, 2017 at 03:56:05PM +0100, Hans Verkuil wrote:
> >> From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> >>
> >> Get the clock for this sensor.
> >>
> >> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> >> ---
> >>  drivers/media/i2c/ov7670.c | 12 ++++++++++++
> >>  1 file changed, 12 insertions(+)
> >>
> >> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> >> index 50e4466a2b37..da0843617a49 100644
> >> --- a/drivers/media/i2c/ov7670.c
> >> +++ b/drivers/media/i2c/ov7670.c
> >> @@ -10,6 +10,7 @@
> >>   * This file may be distributed under the terms of the GNU General
> >>   * Public License, version 2.
> >>   */
> >> +#include <linux/clk.h>
> >>  #include <linux/init.h>
> >>  #include <linux/module.h>
> >>  #include <linux/slab.h>
> >> @@ -227,6 +228,7 @@ struct ov7670_info {
> >>  		struct v4l2_ctrl *hue;
> >>  	};
> >>  	struct ov7670_format_struct *fmt;  /* Current format */
> >> +	struct clk *clk;
> >>  	int min_width;			/* Filter out smaller sizes */
> >>  	int min_height;			/* Filter out smaller sizes */
> >>  	int clock_speed;		/* External clock speed (MHz) */
> >> @@ -1587,6 +1589,15 @@ static int ov7670_probe(struct i2c_client *client,
> >>  			info->pclk_hb_disable = true;
> >>  	}
> >>  
> >> +	info->clk = devm_clk_get(&client->dev, "xclk");
> >> +	if (IS_ERR(info->clk))
> >> +		return -EPROBE_DEFER;
> >> +	clk_prepare_enable(info->clk);
> >> +
> >> +	info->clock_speed = clk_get_rate(info->clk) / 1000000;
> >> +	if (info->clock_speed < 10 || info->clock_speed > 48)
> >> +		return -EINVAL;
> > 
> > clk_disable_unprepare() before return?
> 
> It is my understanding that devm_clk_get will call that for you.
> 
> Correct me if I am wrong.

devm_clk_get() obtained clock is released using devm_clk_release() which is
just calling clk_put(). Which caller prepared or enabled the clock is not
tracked. It's the responsibility of the caller.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 06/15] atmel-isi: document device tree bindings

[Sakari Ailus]

Hi Hans,

On Mon, Mar 06, 2017 at 03:56:07PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> 
> Document the device tree bindings for this hardware.
> 
> Mostly copied from the atmel-isc bindings.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/media/atmel-isi.txt        | 90 +++++++++++++---------
>  1 file changed, 55 insertions(+), 35 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt b/Documentation/devicetree/bindings/media/atmel-isi.txt
> index 251f008f220c..d6e93e8216af 100644
> --- a/Documentation/devicetree/bindings/media/atmel-isi.txt
> +++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
> @@ -1,51 +1,71 @@
> -Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
> -----------------------------------------------
> +Atmel Image Sensor Interface (ISI)
> +----------------------------------
>  
> -Required properties:
> -- compatible: must be "atmel,at91sam9g45-isi"
> -- reg: physical base address and length of the registers set for the device;
> -- interrupts: should contain IRQ line for the ISI;
> -- clocks: list of clock specifiers, corresponding to entries in
> -          the clock-names property;
> -- clock-names: must contain "isi_clk", which is the isi peripherial clock.
> +Required properties for ISI:
> +- compatible: must be "atmel,at91sam9g45-isi".
> +- reg: physical base address and length of the registers set for the device.
> +- interrupts: should contain IRQ line for the ISI.
> +- clocks: list of clock specifiers, corresponding to entries in the clock-names
> +	property; please refer to clock-bindings.txt.
> +- clock-names: required elements: "isi_clk".
> +- pinctrl-names, pinctrl-0: please refer to pinctrl-bindings.txt.
>  
>  ISI supports a single port node with parallel bus. It should contain one

s/should/shall/ ?

>  'port' child node with child 'endpoint' node. Please refer to the bindings
>  defined in Documentation/devicetree/bindings/media/video-interfaces.txt.
>  
> -Example:
> -	isi: isi@f0034000 {
> -		compatible = "atmel,at91sam9g45-isi";
> -		reg = <0xf0034000 0x4000>;
> -		interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
> -
> -		clocks = <&isi_clk>;
> -		clock-names = "isi_clk";
> +Endpoint node properties
> +------------------------
>  
> -		pinctrl-names = "default";
> -		pinctrl-0 = <&pinctrl_isi>;
> +- bus-width: <8> or <10> (mandatory)
> +- hsync-active
> +- vsync-active
> +- pclk-sample
> +- remote-endpoint: A phandle to the bus receiver's endpoint node.

Are these required or are they optional?

The remote-endpoint is at least mandatory, isn't it?

>  
> -		port {
> -			#address-cells = <1>;
> -			#size-cells = <0>;
> +Example:
>  
> -			isi_0: endpoint {
> -				remote-endpoint = <&ov2640_0>;
> -				bus-width = <8>;
> -			};
> +isi: isi@f0034000 {
> +	compatible = "atmel,at91sam9g45-isi";
> +	reg = <0xf0034000 0x4000>;
> +	interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_isi_data_0_7>;
> +	clocks = <&isi_clk>;
> +	clock-names = "isi_clk";
> +	status = "ok";
> +	port {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		isi_0: endpoint {
> +			remote-endpoint = <&ov2640_0>;
> +			bus-width = <8>;
> +			vsync-active = <1>;
> +			hsync-active = <1>;
>  		};
>  	};
> +};
> +
> +i2c1: i2c@f0018000 {
> +	status = "okay";
>  
> -	i2c1: i2c@f0018000 {
> -		ov2640: camera@0x30 {
> -			compatible = "ovti,ov2640";
> -			reg = <0x30>;
> +	ov2640: camera@30 {
> +		compatible = "ovti,ov2640";
> +		reg = <0x30>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_pck0_as_isi_mck &pinctrl_sensor_power &pinctrl_sensor_reset>;
> +		resetb-gpios = <&pioE 11 GPIO_ACTIVE_LOW>;
> +		pwdn-gpios = <&pioE 13 GPIO_ACTIVE_HIGH>;
> +		clocks = <&pck0>;
> +		clock-names = "xvclk";
> +		assigned-clocks = <&pck0>;
> +		assigned-clock-rates = <25000000>;
>  
> -			port {
> -				ov2640_0: endpoint {
> -					remote-endpoint = <&isi_0>;
> -					bus-width = <8>;
> -				};
> +		port {
> +			ov2640_0: endpoint {
> +				remote-endpoint = <&isi_0>;
> +				bus-width = <8>;
>  			};
>  		};
>  	};
> +};

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 05/15] ov7670: add devicetree support

[Hans Verkuil]

On 09/03/17 21:45, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, Mar 06, 2017 at 03:56:06PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Add DT support. Use it to get the reset and pwdn pins (if there are any).
>> Tested with one sensor requiring reset/pwdn and one sensor that doesn't
>> have reset/pwdn pins.
> 
> If I read the datasheet right, lifting the reset signal up will reset the
> sensor but the patch doesn't make use of that, it only ensures the reset
> signal stays low. Should you lift it up for a while as well? The datasheet
> doesn't say for how long that should be done, but that it should be usable
> after 1 ms since pulling reset down.

There does not seem to be any need for that. This sensor also comes in two
models: one with separate pwdn and reset pins, and one where it is just hardwired.

If the hardwired variant doesn't need a reset pulse, then neither does the
variant with pins. It works, and I am not really willing to experiment with this.

Regards,

	Hans

> 
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  drivers/media/i2c/ov7670.c | 40 ++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 38 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
>> index da0843617a49..f9d977430dcf 100644
>> --- a/drivers/media/i2c/ov7670.c
>> +++ b/drivers/media/i2c/ov7670.c
>> @@ -17,6 +17,8 @@
>>  #include <linux/i2c.h>
>>  #include <linux/delay.h>
>>  #include <linux/videodev2.h>
>> +#include <linux/gpio.h>
>> +#include <linux/gpio/consumer.h>
>>  #include <media/v4l2-device.h>
>>  #include <media/v4l2-ctrls.h>
>>  #include <media/v4l2-mediabus.h>
>> @@ -229,6 +231,8 @@ struct ov7670_info {
>>  	};
>>  	struct ov7670_format_struct *fmt;  /* Current format */
>>  	struct clk *clk;
>> +	struct gpio_desc *resetb_gpio;
>> +	struct gpio_desc *pwdn_gpio;
>>  	int min_width;			/* Filter out smaller sizes */
>>  	int min_height;			/* Filter out smaller sizes */
>>  	int clock_speed;		/* External clock speed (MHz) */
>> @@ -591,8 +595,6 @@ static int ov7670_init(struct v4l2_subdev *sd, u32 val)
>>  	return ov7670_write_array(sd, ov7670_default_regs);
>>  }
>>  
>> -
>> -
>>  static int ov7670_detect(struct v4l2_subdev *sd)
>>  {
>>  	unsigned char v;
>> @@ -1549,6 +1551,27 @@ static const struct ov7670_devtype ov7670_devdata[] = {
>>  	},
>>  };
>>  
>> +static int ov7670_init_gpio(struct i2c_client *client, struct ov7670_info *info)
>> +{
>> +	info->pwdn_gpio = devm_gpiod_get_optional(&client->dev, "powerdown",
>> +			GPIOD_OUT_LOW);
>> +	if (IS_ERR(info->pwdn_gpio)) {
>> +		dev_info(&client->dev, "can't get %s GPIO\n", "powerdown");
>> +		return PTR_ERR(info->pwdn_gpio);
>> +	}
>> +
>> +	info->resetb_gpio = devm_gpiod_get_optional(&client->dev, "reset",
>> +			GPIOD_OUT_LOW);
>> +	if (IS_ERR(info->resetb_gpio)) {
>> +		dev_info(&client->dev, "can't get %s GPIO\n", "reset");
>> +		return PTR_ERR(info->resetb_gpio);
>> +	}
>> +
>> +	usleep_range(3000, 5000);
>> +
>> +	return 0;
>> +}
>> +
>>  static int ov7670_probe(struct i2c_client *client,
>>  			const struct i2c_device_id *id)
>>  {
>> @@ -1594,6 +1617,10 @@ static int ov7670_probe(struct i2c_client *client,
>>  		return -EPROBE_DEFER;
>>  	clk_prepare_enable(info->clk);
>>  
>> +	ret = ov7670_init_gpio(client, info);
>> +	if (ret)
>> +		return ret;
>> +
>>  	info->clock_speed = clk_get_rate(info->clk) / 1000000;
>>  	if (info->clock_speed < 10 || info->clock_speed > 48)
>>  		return -EINVAL;
>> @@ -1689,9 +1716,18 @@ static const struct i2c_device_id ov7670_id[] = {
>>  };
>>  MODULE_DEVICE_TABLE(i2c, ov7670_id);
>>  
>> +#if IS_ENABLED(CONFIG_OF)
>> +static const struct of_device_id ov7670_of_match[] = {
>> +	{ .compatible = "ovti,ov7670", },
>> +	{ /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, ov7670_of_match);
>> +#endif
>> +
>>  static struct i2c_driver ov7670_driver = {
>>  	.driver = {
>>  		.name	= "ov7670",
>> +		.of_match_table = of_match_ptr(ov7670_of_match),
>>  	},
>>  	.probe		= ov7670_probe,
>>  	.remove		= ov7670_remove,
> 

Re: [PATCHv3 01/15] ov7670: document device tree bindings

[Hans Verkuil]

On 09/03/17 19:01, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, Mar 06, 2017 at 03:56:02PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Add binding documentation and add that file to the MAINTAINERS entry.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  .../devicetree/bindings/media/i2c/ov7670.txt       | 44 ++++++++++++++++++++++
>>  MAINTAINERS                                        |  1 +
>>  2 files changed, 45 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7670.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov7670.txt b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
>> new file mode 100644
>> index 000000000000..6d9c90dff7a7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
>> @@ -0,0 +1,44 @@
>> +* Omnivision OV7670 CMOS sensor
>> +
>> +The Omnivision OV7670 sensor supports multiple resolutions output, such as
>> +CIF, SVGA, UXGA. It also can support the YUV422/420, RGB565/555 or raw RGB
>> +output formats.
>> +
>> +Required Properties:
>> +- compatible: should be "ovti,ov7670"
>> +- clocks: reference to the xclk input clock.
>> +- clock-names: should be "xclk".
>> +
>> +Optional Properties:
>> +- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
>> +  Active is low.
>> +- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
>> +  Active is high.
>> +
>> +The device node must contain one 'port' child node for its digital output
>> +video port, in accordance with the video interface bindings defined in
>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>> +
>> +Example:
>> +
>> +	i2c1: i2c@f0018000 {
>> +		ov7670: camera@21 {
>> +			compatible = "ovti,ov7670";
>> +			reg = <0x21>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_pck0_as_isi_mck &pinctrl_sensor_power &pinctrl_sensor_reset>;
>> +			reset-gpios = <&pioE 11 GPIO_ACTIVE_LOW>;
>> +			powerdown-gpios = <&pioE 13 GPIO_ACTIVE_HIGH>;
>> +			clocks = <&pck0>;
>> +			clock-names = "xclk";
>> +			assigned-clocks = <&pck0>;
>> +			assigned-clock-rates = <25000000>;
>> +
>> +			port {
>> +				ov7670_0: endpoint {
>> +					remote-endpoint = <&isi_0>;
>> +					bus-width = <8>;
> 
> Didn't I previously request to specify which of the standardised properties
> are relevant for the device (and which ones are required and which are
> optional)? If I didn't, I'm doing that now. :-)
> 
> E.g. the omap3isp driver documentation looks like this:
> 
> Documentation/devicetree/bindings/media/ti,omap3isp.txt

I did document this in the atmel-isi bindings.

The real issue here is that bus-width is simply ignored by the ov7670 driver,
so it likely can be removed from the example in this document.

I clearly state the required and optional properties in this document, it is
just the example that is out-dated.

Regards,

	Hans

> 
>> +				};
>> +			};
>> +		};
>> +	};
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 83a42ef1d1a7..93500928ca4f 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -9273,6 +9273,7 @@ L:	linux-media@vger.kernel.org
>>  T:	git git://linuxtv.org/media_tree.git
>>  S:	Maintained
>>  F:	drivers/media/i2c/ov7670.c
>> +F:	Documentation/devicetree/bindings/media/i2c/ov7670.txt
>>  
>>  ONENAND FLASH DRIVER
>>  M:	Kyungmin Park <kyungmin.park@samsung.com>
> 

Re: [PATCHv3 04/15] ov7670: get xclk

[Hans Verkuil]

On 10/03/17 10:11, Sakari Ailus wrote:
> Hi Hans,
> 
> On Fri, Mar 10, 2017 at 09:55:53AM +0100, Hans Verkuil wrote:
>> On 09/03/17 21:38, Sakari Ailus wrote:
>>> Hi Hans,
>>>
>>> On Mon, Mar 06, 2017 at 03:56:05PM +0100, Hans Verkuil wrote:
>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>>
>>>> Get the clock for this sensor.
>>>>
>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>> ---
>>>>  drivers/media/i2c/ov7670.c | 12 ++++++++++++
>>>>  1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
>>>> index 50e4466a2b37..da0843617a49 100644
>>>> --- a/drivers/media/i2c/ov7670.c
>>>> +++ b/drivers/media/i2c/ov7670.c
>>>> @@ -10,6 +10,7 @@
>>>>   * This file may be distributed under the terms of the GNU General
>>>>   * Public License, version 2.
>>>>   */
>>>> +#include <linux/clk.h>
>>>>  #include <linux/init.h>
>>>>  #include <linux/module.h>
>>>>  #include <linux/slab.h>
>>>> @@ -227,6 +228,7 @@ struct ov7670_info {
>>>>  		struct v4l2_ctrl *hue;
>>>>  	};
>>>>  	struct ov7670_format_struct *fmt;  /* Current format */
>>>> +	struct clk *clk;
>>>>  	int min_width;			/* Filter out smaller sizes */
>>>>  	int min_height;			/* Filter out smaller sizes */
>>>>  	int clock_speed;		/* External clock speed (MHz) */
>>>> @@ -1587,6 +1589,15 @@ static int ov7670_probe(struct i2c_client *client,
>>>>  			info->pclk_hb_disable = true;
>>>>  	}
>>>>  
>>>> +	info->clk = devm_clk_get(&client->dev, "xclk");
>>>> +	if (IS_ERR(info->clk))
>>>> +		return -EPROBE_DEFER;
>>>> +	clk_prepare_enable(info->clk);
>>>> +
>>>> +	info->clock_speed = clk_get_rate(info->clk) / 1000000;
>>>> +	if (info->clock_speed < 10 || info->clock_speed > 48)
>>>> +		return -EINVAL;
>>>
>>> clk_disable_unprepare() before return?
>>
>> It is my understanding that devm_clk_get will call that for you.
>>
>> Correct me if I am wrong.
> 
> devm_clk_get() obtained clock is released using devm_clk_release() which is
> just calling clk_put(). Which caller prepared or enabled the clock is not
> tracked. It's the responsibility of the caller.
> 

You're right. Then I need to check for this.

Hmm, I'm pretty sure I looked at what other drivers do. I think I'm not the
only one who forgets to call clk_disable_unprepare.

Regards,

	Hans

Re: [PATCHv3 07/15] atmel-isi: remove dependency of the soc-camera framework

[Sakari Ailus]

Hi Hans,

Thanks! It's very nice to see one more driver converted to stand-alone one!

Speaking of which --- this seems to be the second last one. The only
remaining one is sh_mobile_ceu_camera.c!

On Mon, Mar 06, 2017 at 03:56:08PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This patch converts the atmel-isi driver from a soc-camera driver to a driver
> that is stand-alone.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/i2c/soc_camera/ov2640.c         |   23 +-
>  drivers/media/platform/soc_camera/Kconfig     |    3 +-
>  drivers/media/platform/soc_camera/atmel-isi.c | 1223 +++++++++++++++----------
>  3 files changed, 735 insertions(+), 514 deletions(-)
> 
> diff --git a/drivers/media/i2c/soc_camera/ov2640.c b/drivers/media/i2c/soc_camera/ov2640.c
> index 56de18263359..b9a0069f5b33 100644
> --- a/drivers/media/i2c/soc_camera/ov2640.c
> +++ b/drivers/media/i2c/soc_camera/ov2640.c

Should this part go to a different patch?

> @@ -794,10 +794,11 @@ static int ov2640_set_params(struct i2c_client *client, u32 *width, u32 *height,
>  		dev_dbg(&client->dev, "%s: Selected cfmt YUYV (YUV422)", __func__);
>  		selected_cfmt_regs = ov2640_yuyv_regs;
>  		break;
> -	default:
>  	case MEDIA_BUS_FMT_UYVY8_2X8:
> +	default:
>  		dev_dbg(&client->dev, "%s: Selected cfmt UYVY", __func__);
>  		selected_cfmt_regs = ov2640_uyvy_regs;
> +		break;
>  	}
>  
>  	/* reset hardware */
> @@ -865,17 +866,7 @@ static int ov2640_get_fmt(struct v4l2_subdev *sd,
>  	mf->width	= priv->win->width;
>  	mf->height	= priv->win->height;
>  	mf->code	= priv->cfmt_code;
> -
> -	switch (mf->code) {
> -	case MEDIA_BUS_FMT_RGB565_2X8_BE:
> -	case MEDIA_BUS_FMT_RGB565_2X8_LE:
> -		mf->colorspace = V4L2_COLORSPACE_SRGB;
> -		break;
> -	default:
> -	case MEDIA_BUS_FMT_YUYV8_2X8:
> -	case MEDIA_BUS_FMT_UYVY8_2X8:
> -		mf->colorspace = V4L2_COLORSPACE_JPEG;
> -	}
> +	mf->colorspace	= V4L2_COLORSPACE_SRGB;
>  	mf->field	= V4L2_FIELD_NONE;
>  
>  	return 0;
> @@ -897,17 +888,17 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
>  	ov2640_select_win(&mf->width, &mf->height);
>  
>  	mf->field	= V4L2_FIELD_NONE;
> +	mf->colorspace	= V4L2_COLORSPACE_SRGB;
>  
>  	switch (mf->code) {
>  	case MEDIA_BUS_FMT_RGB565_2X8_BE:
>  	case MEDIA_BUS_FMT_RGB565_2X8_LE:
> -		mf->colorspace = V4L2_COLORSPACE_SRGB;
> +	case MEDIA_BUS_FMT_YUYV8_2X8:
> +	case MEDIA_BUS_FMT_UYVY8_2X8:
>  		break;
>  	default:
>  		mf->code = MEDIA_BUS_FMT_UYVY8_2X8;
> -	case MEDIA_BUS_FMT_YUYV8_2X8:
> -	case MEDIA_BUS_FMT_UYVY8_2X8:
> -		mf->colorspace = V4L2_COLORSPACE_JPEG;
> +		break;
>  	}
>  
>  	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> diff --git a/drivers/media/platform/soc_camera/Kconfig b/drivers/media/platform/soc_camera/Kconfig
> index 86d74788544f..a37ec91b026e 100644
> --- a/drivers/media/platform/soc_camera/Kconfig
> +++ b/drivers/media/platform/soc_camera/Kconfig
> @@ -29,9 +29,8 @@ config VIDEO_SH_MOBILE_CEU
>  
>  config VIDEO_ATMEL_ISI
>  	tristate "ATMEL Image Sensor Interface (ISI) support"
> -	depends on VIDEO_DEV && SOC_CAMERA
> +	depends on VIDEO_V4L2 && OF && HAS_DMA
>  	depends on ARCH_AT91 || COMPILE_TEST
> -	depends on HAS_DMA
>  	select VIDEOBUF2_DMA_CONTIG
>  	---help---
>  	  This module makes the ATMEL Image Sensor Interface available
> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
> index 46de657c3e6d..a837b94281ef 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
...
> @@ -457,60 +431,83 @@ static void buffer_queue(struct vb2_buffer *vb)
>  		if (vb2_is_streaming(vb->vb2_queue))
>  			start_dma(isi, buf);
>  	}
> -	spin_unlock_irqrestore(&isi->lock, flags);
> +	spin_unlock_irqrestore(&isi->irqlock, flags);
>  }
>  
>  static int start_streaming(struct vb2_queue *vq, unsigned int count)
>  {
> -	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
> -	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
> -	struct atmel_isi *isi = ici->priv;
> +	struct atmel_isi *isi = vb2_get_drv_priv(vq);
> +	struct frame_buffer *buf, *node;
>  	int ret;
>  
> -	pm_runtime_get_sync(ici->v4l2_dev.dev);
> +	/* Enable stream on the sub device */
> +	ret = v4l2_subdev_call(isi->entity.subdev, video, s_stream, 1);
> +	if (ret && ret != -ENOIOCTLCMD) {
> +		v4l2_err(&isi->v4l2_dev, "stream on failed in subdev\n");

You mostly use dev_*() functions to print infromation in the driver. How
about using them consistently? There are few other cases of v4l2_err(), too.

> +		goto err_start_stream;
> +	}
> +
> +	pm_runtime_get_sync(isi->dev);
>  
>  	/* Reset ISI */
>  	ret = atmel_isi_wait_status(isi, WAIT_ISI_RESET);
>  	if (ret < 0) {
> -		dev_err(icd->parent, "Reset ISI timed out\n");
> -		pm_runtime_put(ici->v4l2_dev.dev);
> -		return ret;
> +		dev_err(isi->dev, "Reset ISI timed out\n");
> +		goto err_reset;
>  	}
>  	/* Disable all interrupts */
>  	isi_writel(isi, ISI_INTDIS, (u32)~0UL);
>  
> -	configure_geometry(isi, icd->user_width, icd->user_height,
> -				icd->current_fmt);
> +	isi->sequence = 0;
> +	configure_geometry(isi);
>  
> -	spin_lock_irq(&isi->lock);
> +	spin_lock_irq(&isi->irqlock);
>  	/* Clear any pending interrupt */
>  	isi_readl(isi, ISI_STATUS);
>  
> -	if (count)
> -		start_dma(isi, isi->active);
> -	spin_unlock_irq(&isi->lock);
> +	start_dma(isi, isi->active);
> +	spin_unlock_irq(&isi->irqlock);
>  
>  	return 0;
> +
> +err_reset:
> +	pm_runtime_put(isi->dev);
> +	v4l2_subdev_call(isi->entity.subdev, video, s_stream, 0);
> +
> +err_start_stream:
> +	spin_lock_irq(&isi->irqlock);
> +	isi->active = NULL;
> +	/* Release all active buffers */
> +	list_for_each_entry_safe(buf, node, &isi->video_buffer_list, list) {
> +		list_del_init(&buf->list);
> +		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);
> +	}
> +	spin_unlock_irq(&isi->irqlock);
> +
> +	return ret;
>  }
>  
>  /* abort streaming and wait for last buffer */
>  static void stop_streaming(struct vb2_queue *vq)
>  {
> -	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
> -	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
> -	struct atmel_isi *isi = ici->priv;
> +	struct atmel_isi *isi = vb2_get_drv_priv(vq);
>  	struct frame_buffer *buf, *node;
>  	int ret = 0;
>  	unsigned long timeout;
>  
> -	spin_lock_irq(&isi->lock);
> +	/* Disable stream on the sub device */
> +	ret = v4l2_subdev_call(isi->entity.subdev, video, s_stream, 0);
> +	if (ret && ret != -ENOIOCTLCMD)
> +		v4l2_err(&isi->v4l2_dev, "stream off failed in subdev\n");
> +
> +	spin_lock_irq(&isi->irqlock);
>  	isi->active = NULL;
>  	/* Release all active buffers */
>  	list_for_each_entry_safe(buf, node, &isi->video_buffer_list, list) {
>  		list_del_init(&buf->list);
>  		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
>  	}
> -	spin_unlock_irq(&isi->lock);
> +	spin_unlock_irq(&isi->irqlock);
>  
>  	if (!isi->enable_preview_path) {
>  		timeout = jiffies + FRAME_INTERVAL_MILLI_SEC * HZ;

...

> @@ -1021,13 +864,349 @@ static int atmel_isi_parse_dt(struct atmel_isi *isi,
>  	return 0;
>  }
>  
> +static int isi_open(struct file *file)
> +{
> +	struct atmel_isi *isi = video_drvdata(file);
> +	struct v4l2_subdev *sd = isi->entity.subdev;
> +	int ret;
> +
> +	if (mutex_lock_interruptible(&isi->lock))
> +		return -ERESTARTSYS;
> +
> +	ret = v4l2_fh_open(file);
> +	if (ret < 0)
> +		goto unlock;
> +
> +	if (!v4l2_fh_is_singular_file(file))
> +		goto fh_rel;
> +
> +	ret = v4l2_subdev_call(sd, core, s_power, 1);
> +	if (ret < 0 && ret != -ENOIOCTLCMD)
> +		goto fh_rel;
> +
> +	ret = isi_set_fmt(isi, &isi->fmt);
> +	if (ret)
> +		v4l2_subdev_call(sd, core, s_power, 0);
> +fh_rel:
> +	if (ret)
> +		v4l2_fh_release(file);
> +unlock:
> +	mutex_unlock(&isi->lock);
> +	return ret;
> +}
> +
> +static int isi_release(struct file *file)
> +{
> +	struct atmel_isi *isi = video_drvdata(file);
> +	struct v4l2_subdev *sd = isi->entity.subdev;
> +	bool fh_singular;
> +	int ret;
> +
> +	mutex_lock(&isi->lock);
> +
> +	fh_singular = v4l2_fh_is_singular_file(file);
> +
> +	ret = _vb2_fop_release(file, NULL);
> +
> +	if (fh_singular)
> +		v4l2_subdev_call(sd, core, s_power, 0);
> +
> +	mutex_unlock(&isi->lock);
> +
> +	return ret;
> +}
> +
> +static const struct v4l2_ioctl_ops isi_ioctl_ops = {
> +	.vidioc_querycap		= isi_querycap,
> +
> +	.vidioc_try_fmt_vid_cap		= isi_try_fmt_vid_cap,
> +	.vidioc_g_fmt_vid_cap		= isi_g_fmt_vid_cap,
> +	.vidioc_s_fmt_vid_cap		= isi_s_fmt_vid_cap,
> +	.vidioc_enum_fmt_vid_cap	= isi_enum_fmt_vid_cap,
> +
> +	.vidioc_enum_input		= isi_enum_input,
> +	.vidioc_g_input			= isi_g_input,
> +	.vidioc_s_input			= isi_s_input,
> +
> +	.vidioc_g_parm			= isi_g_parm,
> +	.vidioc_s_parm			= isi_s_parm,
> +	.vidioc_enum_framesizes		= isi_enum_framesizes,
> +	.vidioc_enum_frameintervals	= isi_enum_frameintervals,
> +
> +	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
> +	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
> +	.vidioc_querybuf		= vb2_ioctl_querybuf,
> +	.vidioc_qbuf			= vb2_ioctl_qbuf,
> +	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
> +	.vidioc_expbuf			= vb2_ioctl_expbuf,
> +	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
> +	.vidioc_streamon		= vb2_ioctl_streamon,
> +	.vidioc_streamoff		= vb2_ioctl_streamoff,
> +
> +	.vidioc_log_status		= v4l2_ctrl_log_status,
> +	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,
> +	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
> +};
> +
> +static const struct v4l2_file_operations isi_fops = {
> +	.owner		= THIS_MODULE,
> +	.unlocked_ioctl	= video_ioctl2,
> +	.open		= isi_open,
> +	.release	= isi_release,
> +	.poll		= vb2_fop_poll,
> +	.mmap		= vb2_fop_mmap,
> +	.read		= vb2_fop_read,
> +};
> +
> +static int isi_set_default_fmt(struct atmel_isi *isi)
> +{
> +	struct v4l2_format f = {
> +		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
> +		.fmt.pix = {
> +			.width		= VGA_WIDTH,
> +			.height		= VGA_HEIGHT,
> +			.field		= V4L2_FIELD_NONE,
> +			.pixelformat	= isi->user_formats[0]->fourcc,
> +		},
> +	};
> +	int ret;
> +
> +	ret = isi_try_fmt(isi, &f, NULL);
> +	if (ret)
> +		return ret;
> +	isi->current_fmt = isi->user_formats[0];
> +	isi->fmt = f;
> +	return 0;
> +}
> +
> +static struct isi_format *find_format_by_code(unsigned int code, int *index)
> +{
> +	struct isi_format *fmt = &isi_formats[0];
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(isi_formats); i++) {
> +		if (fmt->mbus_code == code && !fmt->support && !fmt->skip) {
> +			*index = i;
> +			return fmt;
> +		}
> +
> +		fmt++;
> +	}
> +
> +	return NULL;
> +}
> +
> +static int isi_formats_init(struct atmel_isi *isi)
> +{
> +	struct isi_format *fmt;
> +	struct v4l2_subdev *subdev = isi->entity.subdev;
> +	int num_fmts = 0, i, j;
> +	struct v4l2_subdev_mbus_code_enum mbus_code = {
> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> +	};
> +
> +	fmt = &isi_formats[0];
> +
> +	while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
> +	       NULL, &mbus_code)) {
> +		fmt = find_format_by_code(mbus_code.code, &i);
> +		if (!fmt) {
> +			mbus_code.index++;
> +			continue;
> +		}
> +
> +		fmt->support = true;
> +		for (i = 0; i < ARRAY_SIZE(isi_formats); i++)
> +			if (isi_formats[i].fourcc == fmt->fourcc &&
> +			    !isi_formats[i].support)
> +				isi_formats[i].skip = true;
> +		num_fmts++;
> +	}
> +
> +	if (!num_fmts)
> +		return -ENXIO;
> +
> +	isi->num_user_formats = num_fmts;
> +	isi->user_formats = devm_kcalloc(isi->dev,
> +					 num_fmts, sizeof(struct isi_format *),
> +					 GFP_KERNEL);
> +	if (!isi->user_formats) {
> +		v4l2_err(&isi->v4l2_dev, "could not allocate memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	fmt = &isi_formats[0];
> +	for (i = 0, j = 0; i < ARRAY_SIZE(isi_formats); i++) {
> +		if (fmt->support)
> +			isi->user_formats[j++] = fmt;
> +
> +		fmt++;
> +	}
> +	isi->current_fmt = isi->user_formats[0];
> +
> +	return 0;
> +}
> +
> +static int isi_graph_notify_complete(struct v4l2_async_notifier *notifier)
> +{
> +	struct atmel_isi *isi = notifier_to_isi(notifier);
> +	int ret;
> +
> +	isi->vdev->ctrl_handler	= isi->entity.subdev->ctrl_handler;
> +	ret = isi_formats_init(isi);
> +	if (ret) {
> +		dev_err(isi->dev, "No supported mediabus format found\n");
> +		return ret;
> +	}
> +	isi_camera_set_bus_param(isi);
> +
> +	ret = isi_set_default_fmt(isi);
> +	if (ret) {
> +		dev_err(isi->dev, "Could not set default format\n");
> +		return ret;
> +	}
> +
> +	ret = video_register_device(isi->vdev, VFL_TYPE_GRABBER, -1);
> +	if (ret) {
> +		dev_err(isi->dev, "Failed to register video device\n");
> +		return ret;
> +	}
> +
> +	dev_info(isi->dev, "Device registered as %s\n",
> +		 video_device_node_name(isi->vdev));

dev_dbg()?

> +	return 0;
> +}
> +
> +static void isi_graph_notify_unbind(struct v4l2_async_notifier *notifier,
> +				     struct v4l2_subdev *sd,
> +				     struct v4l2_async_subdev *asd)
> +{
> +	struct atmel_isi *isi = notifier_to_isi(notifier);
> +
> +	dev_info(isi->dev, "Removing %s\n",
> +		 video_device_node_name(isi->vdev));

Fits on a single line. dev_dbg()=

> +
> +	/* Checks internaly if vdev have been init or not */
> +	video_unregister_device(isi->vdev);
> +}
> +
> +static int isi_graph_notify_bound(struct v4l2_async_notifier *notifier,
> +				   struct v4l2_subdev *subdev,
> +				   struct v4l2_async_subdev *asd)
> +{
> +	struct atmel_isi *isi = notifier_to_isi(notifier);
> +
> +	dev_dbg(isi->dev, "subdev %s bound\n", subdev->name);
> +
> +	isi->entity.subdev = subdev;
> +
> +	return 0;
> +}
> +
> +static int isi_graph_parse(struct atmel_isi *isi,
> +			    struct device_node *node)

Fits on a single line.

> +{
> +	struct device_node *remote;
> +	struct device_node *ep = NULL;
> +	struct device_node *next;
> +	int ret = 0;
> +
> +	while (1) {
> +		next = of_graph_get_next_endpoint(node, ep);
> +		if (!next)
> +			break;

You could make this a while loop condition.

> +
> +		of_node_put(ep);

ep is put by of_graph_get_next_endpoint(), no need to do it manually here.

> +		ep = next;
> +
> +		remote = of_graph_get_remote_port_parent(ep);
> +		if (!remote) {

of_node_put(ep);

> +			ret = -EINVAL;
> +			break;
> +		}
> +
> +		/* Skip entities that we have already processed. */
> +		if (remote == isi->dev->of_node) {
> +			of_node_put(remote);
> +			continue;
> +		}
> +
> +		/* Remote node to connect */
> +		if (!isi->entity.node) {

There would have to be something wrong about the DT graph for the two above
to happen. I guess one could just return an error if something is terribly
wrong.

I'm thinking this from the point of view of making this function generic,
and moving it to the framework as most drivers to something very similar,
but I'd prefer to get the fwnode patches in first.

> +			isi->entity.node = remote;

You could use entity.asd.match.of.node instead, and drop the node field.

> +			isi->entity.asd.match_type = V4L2_ASYNC_MATCH_OF;
> +			isi->entity.asd.match.of.node = remote;
> +			ret++;
> +		}
> +	}
> +
> +	of_node_put(ep);
> +
> +	return ret;
> +}
> +
> +static int isi_graph_init(struct atmel_isi *isi)
> +{
> +	struct v4l2_async_subdev **subdevs = NULL;
> +	int ret;
> +
> +	/* Parse the graph to extract a list of subdevice DT nodes. */
> +	ret = isi_graph_parse(isi, isi->dev->of_node);
> +	if (ret < 0) {
> +		dev_err(isi->dev, "Graph parsing failed\n");
> +		goto done;
> +	}
> +
> +	if (!ret) {
> +		dev_err(isi->dev, "No subdev found in graph\n");
> +		goto done;
> +	}
> +
> +	if (ret != 1) {
> +		dev_err(isi->dev, "More then one subdev found in graph\n");

Is this an error case? As there's no allocation of memory for subdevs, I
assume so. If multiple devices aren't supported (driver or hardware), how
about flagging this as an error in isi_graph_parse() instead? Putting the
related OF node could be done there, too.

> +		goto done;
> +	}
> +
> +	/* Register the subdevices notifier. */
> +	subdevs = devm_kzalloc(isi->dev, sizeof(*subdevs), GFP_KERNEL);
> +	if (subdevs == NULL) {
> +		ret = -ENOMEM;
> +		goto done;
> +	}
> +
> +	subdevs[0] = &isi->entity.asd;
> +
> +	isi->notifier.subdevs = subdevs;
> +	isi->notifier.num_subdevs = 1;
> +	isi->notifier.bound = isi_graph_notify_bound;
> +	isi->notifier.unbind = isi_graph_notify_unbind;
> +	isi->notifier.complete = isi_graph_notify_complete;
> +
> +	ret = v4l2_async_notifier_register(&isi->v4l2_dev, &isi->notifier);
> +	if (ret < 0) {
> +		dev_err(isi->dev, "Notifier registration failed\n");
> +		goto done;
> +	}
> +
> +	ret = 0;
> +
> +done:
> +	if (ret < 0) {
> +		v4l2_async_notifier_unregister(&isi->notifier);
> +		of_node_put(isi->entity.node);
> +	}
> +
> +	return ret;
> +}
> +
> +
>  static int atmel_isi_probe(struct platform_device *pdev)
>  {
>  	int irq;
>  	struct atmel_isi *isi;
> +	struct vb2_queue *q;
>  	struct resource *regs;
>  	int ret, i;
> -	struct soc_camera_host *soc_host;
>  
>  	isi = devm_kzalloc(&pdev->dev, sizeof(struct atmel_isi), GFP_KERNEL);
>  	if (!isi) {

-- 
Kind regards,

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

Re: [PATCHv3 05/15] ov7670: add devicetree support

[Sakari Ailus]

On Fri, Mar 10, 2017 at 10:32:41AM +0100, Hans Verkuil wrote:
> On 09/03/17 21:45, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > On Mon, Mar 06, 2017 at 03:56:06PM +0100, Hans Verkuil wrote:
> >> From: Hans Verkuil <hans.verkuil@cisco.com>
> >>
> >> Add DT support. Use it to get the reset and pwdn pins (if there are any).
> >> Tested with one sensor requiring reset/pwdn and one sensor that doesn't
> >> have reset/pwdn pins.
> > 
> > If I read the datasheet right, lifting the reset signal up will reset the
> > sensor but the patch doesn't make use of that, it only ensures the reset
> > signal stays low. Should you lift it up for a while as well? The datasheet
> > doesn't say for how long that should be done, but that it should be usable
> > after 1 ms since pulling reset down.
> 
> There does not seem to be any need for that. This sensor also comes in two
> models: one with separate pwdn and reset pins, and one where it is just hardwired.
> 
> If the hardwired variant doesn't need a reset pulse, then neither does the
> variant with pins. It works, and I am not really willing to experiment with this.

Fine with me.

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

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

Re: [PATCHv3 08/15] atmel-isi: move out of soc_camera to atmel

[Sakari Ailus]

On Mon, Mar 06, 2017 at 03:56:09PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> 
> Move this out of the soc_camera directory into the atmel directory
> where it belongs.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

Acked-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

-- 
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 13/15] em28xx: drop last soc_camera link

[Sakari Ailus]

On Mon, Mar 06, 2017 at 03:56:14PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> 
> The em28xx driver still used the soc_camera.h header for the ov2640
> driver. Since this driver no longer uses soc_camera, that include can
> be removed.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

Acked-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

-- 
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 07/15] atmel-isi: remove dependency of the soc-camera framework

[Hans Verkuil]

On 10/03/17 11:39, Sakari Ailus wrote:
> Hi Hans,
> 
> Thanks! It's very nice to see one more driver converted to stand-alone one!
> 
> Speaking of which --- this seems to be the second last one. The only
> remaining one is sh_mobile_ceu_camera.c!
> 
> On Mon, Mar 06, 2017 at 03:56:08PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> This patch converts the atmel-isi driver from a soc-camera driver to a driver
>> that is stand-alone.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  drivers/media/i2c/soc_camera/ov2640.c         |   23 +-
>>  drivers/media/platform/soc_camera/Kconfig     |    3 +-
>>  drivers/media/platform/soc_camera/atmel-isi.c | 1223 +++++++++++++++----------
>>  3 files changed, 735 insertions(+), 514 deletions(-)
>>
>> diff --git a/drivers/media/i2c/soc_camera/ov2640.c b/drivers/media/i2c/soc_camera/ov2640.c
>> index 56de18263359..b9a0069f5b33 100644
>> --- a/drivers/media/i2c/soc_camera/ov2640.c
>> +++ b/drivers/media/i2c/soc_camera/ov2640.c
> 
> Should this part go to a different patch?

Oops, yes. How did this end up here?

Split this off as a separate ov2640 patch.

> 
>> @@ -794,10 +794,11 @@ static int ov2640_set_params(struct i2c_client *client, u32 *width, u32 *height,
>>  		dev_dbg(&client->dev, "%s: Selected cfmt YUYV (YUV422)", __func__);
>>  		selected_cfmt_regs = ov2640_yuyv_regs;
>>  		break;
>> -	default:
>>  	case MEDIA_BUS_FMT_UYVY8_2X8:
>> +	default:
>>  		dev_dbg(&client->dev, "%s: Selected cfmt UYVY", __func__);
>>  		selected_cfmt_regs = ov2640_uyvy_regs;
>> +		break;
>>  	}
>>  
>>  	/* reset hardware */
>> @@ -865,17 +866,7 @@ static int ov2640_get_fmt(struct v4l2_subdev *sd,
>>  	mf->width	= priv->win->width;
>>  	mf->height	= priv->win->height;
>>  	mf->code	= priv->cfmt_code;
>> -
>> -	switch (mf->code) {
>> -	case MEDIA_BUS_FMT_RGB565_2X8_BE:
>> -	case MEDIA_BUS_FMT_RGB565_2X8_LE:
>> -		mf->colorspace = V4L2_COLORSPACE_SRGB;
>> -		break;
>> -	default:
>> -	case MEDIA_BUS_FMT_YUYV8_2X8:
>> -	case MEDIA_BUS_FMT_UYVY8_2X8:
>> -		mf->colorspace = V4L2_COLORSPACE_JPEG;
>> -	}
>> +	mf->colorspace	= V4L2_COLORSPACE_SRGB;
>>  	mf->field	= V4L2_FIELD_NONE;
>>  
>>  	return 0;
>> @@ -897,17 +888,17 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
>>  	ov2640_select_win(&mf->width, &mf->height);
>>  
>>  	mf->field	= V4L2_FIELD_NONE;
>> +	mf->colorspace	= V4L2_COLORSPACE_SRGB;
>>  
>>  	switch (mf->code) {
>>  	case MEDIA_BUS_FMT_RGB565_2X8_BE:
>>  	case MEDIA_BUS_FMT_RGB565_2X8_LE:
>> -		mf->colorspace = V4L2_COLORSPACE_SRGB;
>> +	case MEDIA_BUS_FMT_YUYV8_2X8:
>> +	case MEDIA_BUS_FMT_UYVY8_2X8:
>>  		break;
>>  	default:
>>  		mf->code = MEDIA_BUS_FMT_UYVY8_2X8;
>> -	case MEDIA_BUS_FMT_YUYV8_2X8:
>> -	case MEDIA_BUS_FMT_UYVY8_2X8:
>> -		mf->colorspace = V4L2_COLORSPACE_JPEG;
>> +		break;
>>  	}
>>  
>>  	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>> diff --git a/drivers/media/platform/soc_camera/Kconfig b/drivers/media/platform/soc_camera/Kconfig
>> index 86d74788544f..a37ec91b026e 100644
>> --- a/drivers/media/platform/soc_camera/Kconfig
>> +++ b/drivers/media/platform/soc_camera/Kconfig
>> @@ -29,9 +29,8 @@ config VIDEO_SH_MOBILE_CEU
>>  
>>  config VIDEO_ATMEL_ISI
>>  	tristate "ATMEL Image Sensor Interface (ISI) support"
>> -	depends on VIDEO_DEV && SOC_CAMERA
>> +	depends on VIDEO_V4L2 && OF && HAS_DMA
>>  	depends on ARCH_AT91 || COMPILE_TEST
>> -	depends on HAS_DMA
>>  	select VIDEOBUF2_DMA_CONTIG
>>  	---help---
>>  	  This module makes the ATMEL Image Sensor Interface available
>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
>> index 46de657c3e6d..a837b94281ef 100644
>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> ...
>> @@ -457,60 +431,83 @@ static void buffer_queue(struct vb2_buffer *vb)
>>  		if (vb2_is_streaming(vb->vb2_queue))
>>  			start_dma(isi, buf);
>>  	}
>> -	spin_unlock_irqrestore(&isi->lock, flags);
>> +	spin_unlock_irqrestore(&isi->irqlock, flags);
>>  }
>>  
>>  static int start_streaming(struct vb2_queue *vq, unsigned int count)
>>  {
>> -	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
>> -	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
>> -	struct atmel_isi *isi = ici->priv;
>> +	struct atmel_isi *isi = vb2_get_drv_priv(vq);
>> +	struct frame_buffer *buf, *node;
>>  	int ret;
>>  
>> -	pm_runtime_get_sync(ici->v4l2_dev.dev);
>> +	/* Enable stream on the sub device */
>> +	ret = v4l2_subdev_call(isi->entity.subdev, video, s_stream, 1);
>> +	if (ret && ret != -ENOIOCTLCMD) {
>> +		v4l2_err(&isi->v4l2_dev, "stream on failed in subdev\n");
> 
> You mostly use dev_*() functions to print infromation in the driver. How
> about using them consistently? There are few other cases of v4l2_err(), too.

Converted them all to dev_err.

> 
>> +		goto err_start_stream;
>> +	}
>> +
>> +	pm_runtime_get_sync(isi->dev);
>>  
>>  	/* Reset ISI */
>>  	ret = atmel_isi_wait_status(isi, WAIT_ISI_RESET);
>>  	if (ret < 0) {
>> -		dev_err(icd->parent, "Reset ISI timed out\n");
>> -		pm_runtime_put(ici->v4l2_dev.dev);
>> -		return ret;
>> +		dev_err(isi->dev, "Reset ISI timed out\n");
>> +		goto err_reset;
>>  	}
>>  	/* Disable all interrupts */
>>  	isi_writel(isi, ISI_INTDIS, (u32)~0UL);
>>  
>> -	configure_geometry(isi, icd->user_width, icd->user_height,
>> -				icd->current_fmt);
>> +	isi->sequence = 0;
>> +	configure_geometry(isi);
>>  
>> -	spin_lock_irq(&isi->lock);
>> +	spin_lock_irq(&isi->irqlock);
>>  	/* Clear any pending interrupt */
>>  	isi_readl(isi, ISI_STATUS);
>>  
>> -	if (count)
>> -		start_dma(isi, isi->active);
>> -	spin_unlock_irq(&isi->lock);
>> +	start_dma(isi, isi->active);
>> +	spin_unlock_irq(&isi->irqlock);
>>  
>>  	return 0;
>> +
>> +err_reset:
>> +	pm_runtime_put(isi->dev);
>> +	v4l2_subdev_call(isi->entity.subdev, video, s_stream, 0);
>> +
>> +err_start_stream:
>> +	spin_lock_irq(&isi->irqlock);
>> +	isi->active = NULL;
>> +	/* Release all active buffers */
>> +	list_for_each_entry_safe(buf, node, &isi->video_buffer_list, list) {
>> +		list_del_init(&buf->list);
>> +		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);
>> +	}
>> +	spin_unlock_irq(&isi->irqlock);
>> +
>> +	return ret;
>>  }
>>  
>>  /* abort streaming and wait for last buffer */
>>  static void stop_streaming(struct vb2_queue *vq)
>>  {
>> -	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
>> -	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
>> -	struct atmel_isi *isi = ici->priv;
>> +	struct atmel_isi *isi = vb2_get_drv_priv(vq);
>>  	struct frame_buffer *buf, *node;
>>  	int ret = 0;
>>  	unsigned long timeout;
>>  
>> -	spin_lock_irq(&isi->lock);
>> +	/* Disable stream on the sub device */
>> +	ret = v4l2_subdev_call(isi->entity.subdev, video, s_stream, 0);
>> +	if (ret && ret != -ENOIOCTLCMD)
>> +		v4l2_err(&isi->v4l2_dev, "stream off failed in subdev\n");
>> +
>> +	spin_lock_irq(&isi->irqlock);
>>  	isi->active = NULL;
>>  	/* Release all active buffers */
>>  	list_for_each_entry_safe(buf, node, &isi->video_buffer_list, list) {
>>  		list_del_init(&buf->list);
>>  		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
>>  	}
>> -	spin_unlock_irq(&isi->lock);
>> +	spin_unlock_irq(&isi->irqlock);
>>  
>>  	if (!isi->enable_preview_path) {
>>  		timeout = jiffies + FRAME_INTERVAL_MILLI_SEC * HZ;
> 
> ...
> 
>> @@ -1021,13 +864,349 @@ static int atmel_isi_parse_dt(struct atmel_isi *isi,
>>  	return 0;
>>  }
>>  
>> +static int isi_open(struct file *file)
>> +{
>> +	struct atmel_isi *isi = video_drvdata(file);
>> +	struct v4l2_subdev *sd = isi->entity.subdev;
>> +	int ret;
>> +
>> +	if (mutex_lock_interruptible(&isi->lock))
>> +		return -ERESTARTSYS;
>> +
>> +	ret = v4l2_fh_open(file);
>> +	if (ret < 0)
>> +		goto unlock;
>> +
>> +	if (!v4l2_fh_is_singular_file(file))
>> +		goto fh_rel;
>> +
>> +	ret = v4l2_subdev_call(sd, core, s_power, 1);
>> +	if (ret < 0 && ret != -ENOIOCTLCMD)
>> +		goto fh_rel;
>> +
>> +	ret = isi_set_fmt(isi, &isi->fmt);
>> +	if (ret)
>> +		v4l2_subdev_call(sd, core, s_power, 0);
>> +fh_rel:
>> +	if (ret)
>> +		v4l2_fh_release(file);
>> +unlock:
>> +	mutex_unlock(&isi->lock);
>> +	return ret;
>> +}
>> +
>> +static int isi_release(struct file *file)
>> +{
>> +	struct atmel_isi *isi = video_drvdata(file);
>> +	struct v4l2_subdev *sd = isi->entity.subdev;
>> +	bool fh_singular;
>> +	int ret;
>> +
>> +	mutex_lock(&isi->lock);
>> +
>> +	fh_singular = v4l2_fh_is_singular_file(file);
>> +
>> +	ret = _vb2_fop_release(file, NULL);
>> +
>> +	if (fh_singular)
>> +		v4l2_subdev_call(sd, core, s_power, 0);
>> +
>> +	mutex_unlock(&isi->lock);
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct v4l2_ioctl_ops isi_ioctl_ops = {
>> +	.vidioc_querycap		= isi_querycap,
>> +
>> +	.vidioc_try_fmt_vid_cap		= isi_try_fmt_vid_cap,
>> +	.vidioc_g_fmt_vid_cap		= isi_g_fmt_vid_cap,
>> +	.vidioc_s_fmt_vid_cap		= isi_s_fmt_vid_cap,
>> +	.vidioc_enum_fmt_vid_cap	= isi_enum_fmt_vid_cap,
>> +
>> +	.vidioc_enum_input		= isi_enum_input,
>> +	.vidioc_g_input			= isi_g_input,
>> +	.vidioc_s_input			= isi_s_input,
>> +
>> +	.vidioc_g_parm			= isi_g_parm,
>> +	.vidioc_s_parm			= isi_s_parm,
>> +	.vidioc_enum_framesizes		= isi_enum_framesizes,
>> +	.vidioc_enum_frameintervals	= isi_enum_frameintervals,
>> +
>> +	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
>> +	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
>> +	.vidioc_querybuf		= vb2_ioctl_querybuf,
>> +	.vidioc_qbuf			= vb2_ioctl_qbuf,
>> +	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
>> +	.vidioc_expbuf			= vb2_ioctl_expbuf,
>> +	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
>> +	.vidioc_streamon		= vb2_ioctl_streamon,
>> +	.vidioc_streamoff		= vb2_ioctl_streamoff,
>> +
>> +	.vidioc_log_status		= v4l2_ctrl_log_status,
>> +	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,
>> +	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
>> +};
>> +
>> +static const struct v4l2_file_operations isi_fops = {
>> +	.owner		= THIS_MODULE,
>> +	.unlocked_ioctl	= video_ioctl2,
>> +	.open		= isi_open,
>> +	.release	= isi_release,
>> +	.poll		= vb2_fop_poll,
>> +	.mmap		= vb2_fop_mmap,
>> +	.read		= vb2_fop_read,
>> +};
>> +
>> +static int isi_set_default_fmt(struct atmel_isi *isi)
>> +{
>> +	struct v4l2_format f = {
>> +		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
>> +		.fmt.pix = {
>> +			.width		= VGA_WIDTH,
>> +			.height		= VGA_HEIGHT,
>> +			.field		= V4L2_FIELD_NONE,
>> +			.pixelformat	= isi->user_formats[0]->fourcc,
>> +		},
>> +	};
>> +	int ret;
>> +
>> +	ret = isi_try_fmt(isi, &f, NULL);
>> +	if (ret)
>> +		return ret;
>> +	isi->current_fmt = isi->user_formats[0];
>> +	isi->fmt = f;
>> +	return 0;
>> +}
>> +
>> +static struct isi_format *find_format_by_code(unsigned int code, int *index)
>> +{
>> +	struct isi_format *fmt = &isi_formats[0];
>> +	unsigned int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(isi_formats); i++) {
>> +		if (fmt->mbus_code == code && !fmt->support && !fmt->skip) {
>> +			*index = i;
>> +			return fmt;
>> +		}
>> +
>> +		fmt++;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>> +static int isi_formats_init(struct atmel_isi *isi)
>> +{
>> +	struct isi_format *fmt;
>> +	struct v4l2_subdev *subdev = isi->entity.subdev;
>> +	int num_fmts = 0, i, j;
>> +	struct v4l2_subdev_mbus_code_enum mbus_code = {
>> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
>> +	};
>> +
>> +	fmt = &isi_formats[0];
>> +
>> +	while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
>> +	       NULL, &mbus_code)) {
>> +		fmt = find_format_by_code(mbus_code.code, &i);
>> +		if (!fmt) {
>> +			mbus_code.index++;
>> +			continue;
>> +		}
>> +
>> +		fmt->support = true;
>> +		for (i = 0; i < ARRAY_SIZE(isi_formats); i++)
>> +			if (isi_formats[i].fourcc == fmt->fourcc &&
>> +			    !isi_formats[i].support)
>> +				isi_formats[i].skip = true;
>> +		num_fmts++;
>> +	}
>> +
>> +	if (!num_fmts)
>> +		return -ENXIO;
>> +
>> +	isi->num_user_formats = num_fmts;
>> +	isi->user_formats = devm_kcalloc(isi->dev,
>> +					 num_fmts, sizeof(struct isi_format *),
>> +					 GFP_KERNEL);
>> +	if (!isi->user_formats) {
>> +		v4l2_err(&isi->v4l2_dev, "could not allocate memory\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	fmt = &isi_formats[0];
>> +	for (i = 0, j = 0; i < ARRAY_SIZE(isi_formats); i++) {
>> +		if (fmt->support)
>> +			isi->user_formats[j++] = fmt;
>> +
>> +		fmt++;
>> +	}
>> +	isi->current_fmt = isi->user_formats[0];
>> +
>> +	return 0;
>> +}
>> +
>> +static int isi_graph_notify_complete(struct v4l2_async_notifier *notifier)
>> +{
>> +	struct atmel_isi *isi = notifier_to_isi(notifier);
>> +	int ret;
>> +
>> +	isi->vdev->ctrl_handler	= isi->entity.subdev->ctrl_handler;
>> +	ret = isi_formats_init(isi);
>> +	if (ret) {
>> +		dev_err(isi->dev, "No supported mediabus format found\n");
>> +		return ret;
>> +	}
>> +	isi_camera_set_bus_param(isi);
>> +
>> +	ret = isi_set_default_fmt(isi);
>> +	if (ret) {
>> +		dev_err(isi->dev, "Could not set default format\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = video_register_device(isi->vdev, VFL_TYPE_GRABBER, -1);
>> +	if (ret) {
>> +		dev_err(isi->dev, "Failed to register video device\n");
>> +		return ret;
>> +	}
>> +
>> +	dev_info(isi->dev, "Device registered as %s\n",
>> +		 video_device_node_name(isi->vdev));
> 
> dev_dbg()?
> 
>> +	return 0;
>> +}
>> +
>> +static void isi_graph_notify_unbind(struct v4l2_async_notifier *notifier,
>> +				     struct v4l2_subdev *sd,
>> +				     struct v4l2_async_subdev *asd)
>> +{
>> +	struct atmel_isi *isi = notifier_to_isi(notifier);
>> +
>> +	dev_info(isi->dev, "Removing %s\n",
>> +		 video_device_node_name(isi->vdev));
> 
> Fits on a single line. dev_dbg()=
> 
>> +
>> +	/* Checks internaly if vdev have been init or not */
>> +	video_unregister_device(isi->vdev);
>> +}
>> +
>> +static int isi_graph_notify_bound(struct v4l2_async_notifier *notifier,
>> +				   struct v4l2_subdev *subdev,
>> +				   struct v4l2_async_subdev *asd)
>> +{
>> +	struct atmel_isi *isi = notifier_to_isi(notifier);
>> +
>> +	dev_dbg(isi->dev, "subdev %s bound\n", subdev->name);
>> +
>> +	isi->entity.subdev = subdev;
>> +
>> +	return 0;
>> +}
>> +
>> +static int isi_graph_parse(struct atmel_isi *isi,
>> +			    struct device_node *node)
> 
> Fits on a single line.
> 
>> +{
>> +	struct device_node *remote;
>> +	struct device_node *ep = NULL;
>> +	struct device_node *next;
>> +	int ret = 0;
>> +
>> +	while (1) {
>> +		next = of_graph_get_next_endpoint(node, ep);
>> +		if (!next)
>> +			break;
> 
> You could make this a while loop condition.

You would need to do:

	while (next = of_graph_get_next_endpoint(node, ep))

but assignments in a condition are frowned upon.

I'll keep this as is.

> 
>> +
>> +		of_node_put(ep);
> 
> ep is put by of_graph_get_next_endpoint(), no need to do it manually here.
> 
>> +		ep = next;
>> +
>> +		remote = of_graph_get_remote_port_parent(ep);
>> +		if (!remote) {
> 
> of_node_put(ep);
> 
>> +			ret = -EINVAL;
>> +			break;
>> +		}
>> +
>> +		/* Skip entities that we have already processed. */
>> +		if (remote == isi->dev->of_node) {
>> +			of_node_put(remote);
>> +			continue;
>> +		}
>> +
>> +		/* Remote node to connect */
>> +		if (!isi->entity.node) {
> 
> There would have to be something wrong about the DT graph for the two above
> to happen. I guess one could just return an error if something is terribly
> wrong.
> 
> I'm thinking this from the point of view of making this function generic,
> and moving it to the framework as most drivers to something very similar,
> but I'd prefer to get the fwnode patches in first.

I just dropped these tests. I agree that they don't make a lot of sense.

> 
>> +			isi->entity.node = remote;
> 
> You could use entity.asd.match.of.node instead, and drop the node field.

Done.

> 
>> +			isi->entity.asd.match_type = V4L2_ASYNC_MATCH_OF;
>> +			isi->entity.asd.match.of.node = remote;
>> +			ret++;
>> +		}
>> +	}
>> +
>> +	of_node_put(ep);
>> +
>> +	return ret;
>> +}

Please check the upcoming new version of this code if you agree with it.

>> +
>> +static int isi_graph_init(struct atmel_isi *isi)
>> +{
>> +	struct v4l2_async_subdev **subdevs = NULL;
>> +	int ret;
>> +
>> +	/* Parse the graph to extract a list of subdevice DT nodes. */
>> +	ret = isi_graph_parse(isi, isi->dev->of_node);
>> +	if (ret < 0) {
>> +		dev_err(isi->dev, "Graph parsing failed\n");
>> +		goto done;
>> +	}
>> +
>> +	if (!ret) {
>> +		dev_err(isi->dev, "No subdev found in graph\n");
>> +		goto done;
>> +	}
>> +
>> +	if (ret != 1) {
>> +		dev_err(isi->dev, "More then one subdev found in graph\n");
> 
> Is this an error case? As there's no allocation of memory for subdevs, I
> assume so. If multiple devices aren't supported (driver or hardware), how
> about flagging this as an error in isi_graph_parse() instead? Putting the
> related OF node could be done there, too.

Done.

> 
>> +		goto done;
>> +	}
>> +
>> +	/* Register the subdevices notifier. */
>> +	subdevs = devm_kzalloc(isi->dev, sizeof(*subdevs), GFP_KERNEL);
>> +	if (subdevs == NULL) {
>> +		ret = -ENOMEM;
>> +		goto done;
>> +	}
>> +
>> +	subdevs[0] = &isi->entity.asd;
>> +
>> +	isi->notifier.subdevs = subdevs;
>> +	isi->notifier.num_subdevs = 1;
>> +	isi->notifier.bound = isi_graph_notify_bound;
>> +	isi->notifier.unbind = isi_graph_notify_unbind;
>> +	isi->notifier.complete = isi_graph_notify_complete;
>> +
>> +	ret = v4l2_async_notifier_register(&isi->v4l2_dev, &isi->notifier);
>> +	if (ret < 0) {
>> +		dev_err(isi->dev, "Notifier registration failed\n");
>> +		goto done;
>> +	}
>> +
>> +	ret = 0;
>> +
>> +done:
>> +	if (ret < 0) {
>> +		v4l2_async_notifier_unregister(&isi->notifier);
>> +		of_node_put(isi->entity.node);
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +
>>  static int atmel_isi_probe(struct platform_device *pdev)
>>  {
>>  	int irq;
>>  	struct atmel_isi *isi;
>> +	struct vb2_queue *q;
>>  	struct resource *regs;
>>  	int ret, i;
>> -	struct soc_camera_host *soc_host;
>>  
>>  	isi = devm_kzalloc(&pdev->dev, sizeof(struct atmel_isi), GFP_KERNEL);
>>  	if (!isi) {
> 

Regards,

	Hans

Re: [PATCHv3 07/15] atmel-isi: remove dependency of the soc-camera framework

[Hans Verkuil]

On 10/03/17 11:39, Sakari Ailus wrote:
> Hi Hans,
> 
> Thanks! It's very nice to see one more driver converted to stand-alone one!
> 
> Speaking of which --- this seems to be the second last one. The only
> remaining one is sh_mobile_ceu_camera.c!
> 
> On Mon, Mar 06, 2017 at 03:56:08PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
>>
>> This patch converts the atmel-isi driver from a soc-camera driver to a driver
>> that is stand-alone.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
>> ---
>>  drivers/media/i2c/soc_camera/ov2640.c         |   23 +-
>>  drivers/media/platform/soc_camera/Kconfig     |    3 +-
>>  drivers/media/platform/soc_camera/atmel-isi.c | 1223 +++++++++++++++----------
>>  3 files changed, 735 insertions(+), 514 deletions(-)
>>

<snip>

>> +static int isi_graph_parse(struct atmel_isi *isi,
>> +			    struct device_node *node)
> 
> Fits on a single line.
> 
>> +{
>> +	struct device_node *remote;
>> +	struct device_node *ep = NULL;
>> +	struct device_node *next;
>> +	int ret = 0;
>> +
>> +	while (1) {
>> +		next = of_graph_get_next_endpoint(node, ep);
>> +		if (!next)
>> +			break;
> 
> You could make this a while loop condition.
> 
>> +
>> +		of_node_put(ep);
> 
> ep is put by of_graph_get_next_endpoint(), no need to do it manually here.
> 
>> +		ep = next;
>> +
>> +		remote = of_graph_get_remote_port_parent(ep);
>> +		if (!remote) {
> 
> of_node_put(ep);
> 
>> +			ret = -EINVAL;
>> +			break;
>> +		}
>> +
>> +		/* Skip entities that we have already processed. */
>> +		if (remote == isi->dev->of_node) {
>> +			of_node_put(remote);
>> +			continue;
>> +		}
>> +
>> +		/* Remote node to connect */
>> +		if (!isi->entity.node) {
> 
> There would have to be something wrong about the DT graph for the two above
> to happen. I guess one could just return an error if something is terribly
> wrong.
> 
> I'm thinking this from the point of view of making this function generic,
> and moving it to the framework as most drivers to something very similar,
> but I'd prefer to get the fwnode patches in first.
> 
>> +			isi->entity.node = remote;
> 
> You could use entity.asd.match.of.node instead, and drop the node field.

Slight problem with this. If I make this change, then the of_node_put below
changes as well:

@@ -1193,7 +1176,7 @@ static int isi_graph_init(struct atmel_isi *isi)
 done:
        if (ret < 0) {
                v4l2_async_notifier_unregister(&isi->notifier);
-               of_node_put(isi->entity.node);
+               of_node_put(isi->entity.asd.match.of.node);
        }

And I get this compiler warning:

  CC [M]  drivers/media/platform/atmel/atmel-isi.o
drivers/media/platform/atmel/atmel-isi.c: In function ‘isi_graph_init’:
drivers/media/platform/atmel/atmel-isi.c:1179:15: warning: passing argument 1 of ‘of_node_put’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
   of_node_put(isi->entity.asd.match.of.node);
               ^~~
In file included from drivers/media/platform/atmel/atmel-isi.c:25:0:
./include/linux/of.h:130:20: note: expected ‘struct device_node *’ but argument is of type ‘const struct device_node *’
 static inline void of_node_put(struct device_node *node) { }
                    ^~~~~~~~~~~


Any suggestions? Just keep the entity.node after all?

Regards,

	Hans

> 
>> +			isi->entity.asd.match_type = V4L2_ASYNC_MATCH_OF;
>> +			isi->entity.asd.match.of.node = remote;
>> +			ret++;
>> +		}
>> +	}
>> +
>> +	of_node_put(ep);
>> +
>> +	return ret;
>> +}


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 07/15] atmel-isi: remove dependency of the soc-camera framework

[Sakari Ailus]

Hi Hans,

On Fri, Mar 10, 2017 at 12:25:54PM +0100, Hans Verkuil wrote:
> Slight problem with this. If I make this change, then the of_node_put below
> changes as well:
> 
> @@ -1193,7 +1176,7 @@ static int isi_graph_init(struct atmel_isi *isi)
>  done:
>         if (ret < 0) {
>                 v4l2_async_notifier_unregister(&isi->notifier);
> -               of_node_put(isi->entity.node);
> +               of_node_put(isi->entity.asd.match.of.node);
>         }
> 
> And I get this compiler warning:
> 
>   CC [M]  drivers/media/platform/atmel/atmel-isi.o
> drivers/media/platform/atmel/atmel-isi.c: In function ‘isi_graph_init’:
> drivers/media/platform/atmel/atmel-isi.c:1179:15: warning: passing argument 1 of ‘of_node_put’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
>    of_node_put(isi->entity.asd.match.of.node);
>                ^~~
> In file included from drivers/media/platform/atmel/atmel-isi.c:25:0:
> ./include/linux/of.h:130:20: note: expected ‘struct device_node *’ but argument is of type ‘const struct device_node *’
>  static inline void of_node_put(struct device_node *node) { }
>                     ^~~~~~~~~~~
> 
> 
> Any suggestions? Just keep the entity.node after all?

Yeah, makes sense, I didn't come to think of that.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html