[PATCHv5 00/16] atmel-isi/ov7670/ov2640: convert to standalone drivers

[PATCHv5 00/16] 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 6 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 v4:
- the ov2640 colorspace fixes were inexplicably part of an atmel-isi patch.
  Split it off as a separate patch.
- add V4L2_SUBDEV_FL_HAS_DEVNODE to ov2640.
- drop #if defined(CONFIG_MEDIA_CONTROLLER) guard around media_entity_cleanup
  in ov2640.

Changes since v3:
- ov2640/ov7670: call clk_disable_unprepare where needed. I assumed this was
  done by the devm_clk_get cleanup, but that wasn't the case.
- bindings: be even more explicit about which properties are mandatory.
- ov2640/ov7670: drop unused bus-width from the dts binding examples and from
  the actual dts patches.

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 (16):
  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: fix colorspace handling
  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        |   96 +-
 .../devicetree/bindings/media/i2c/ov2640.txt       |   23 +-
 .../devicetree/bindings/media/i2c/ov7670.txt       |   43 +
 MAINTAINERS                                        |    1 +
 arch/arm/boot/dts/at91-sama5d3_xplained.dts        |   59 +-
 arch/arm/boot/dts/sama5d3.dtsi                     |    4 +-
 drivers/media/i2c/Kconfig                          |   11 +
 drivers/media/i2c/Makefile                         |    1 +
 drivers/media/i2c/{soc_camera => }/ov2640.c        |  149 +--
 drivers/media/i2c/ov7670.c                         |   75 +-
 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           | 1384 ++++++++++++++++++++
 .../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, 1687 insertions(+), 1367 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

[PATCHv5 01/16] 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>
Acked-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 .../devicetree/bindings/media/i2c/ov7670.txt       | 43 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 44 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..826b6563b009
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
@@ -0,0 +1,43 @@
+* 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>;
+				};
+			};
+		};
+	};
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

Re: [PATCHv5 01/16] ov7670: document device tree bindings

[Rob Herring]

On Sat, Mar 11, 2017 at 12:23:13PM +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>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  .../devicetree/bindings/media/i2c/ov7670.txt       | 43 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7670.txt

Acked-by: Rob Herring <robh@kernel.org>

[PATCHv5 05/16] 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>
Acked-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@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 912ff09c6100..7270c68ed18a 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)
+		goto clk_disable;
+
 	info->clock_speed = clk_get_rate(info->clk) / 1000000;
 	if (info->clock_speed < 10 || info->clock_speed > 48) {
 		ret = -EINVAL;
@@ -1693,9 +1720,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

[PATCHv5 11/16] ov2640: convert from soc-camera to a standard subdev sensor driver.

[Hans Verkuil]

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

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-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
Acked-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 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

--
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

[PATCHv5 13/16] 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 | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index 0445963c5fae..d1f04a4ca2ac 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;
@@ -1063,6 +1066,7 @@ static int ov2640_probe(struct i2c_client *client,
 		goto err_clk;
 
 	v4l2_i2c_subdev_init(&priv->subdev, client, &ov2640_subdev_ops);
+	priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
 	v4l2_ctrl_handler_init(&priv->hdl, 2);
 	v4l2_ctrl_new_std(&priv->hdl, &ov2640_ctrl_ops,
 			V4L2_CID_VFLIP, 0, 1, 1, 0);
@@ -1073,19 +1077,28 @@ 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:
+	media_entity_cleanup(&priv->subdev.entity);
 err_hdl:
 	v4l2_ctrl_handler_free(&priv->hdl);
 err_clk:
@@ -1099,6 +1112,7 @@ static int ov2640_remove(struct i2c_client *client)
 
 	v4l2_async_unregister_subdev(&priv->subdev);
 	v4l2_ctrl_handler_free(&priv->hdl);
+	media_entity_cleanup(&priv->subdev.entity);
 	v4l2_device_unregister_subdev(&priv->subdev);
 	clk_disable_unprepare(priv->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

[PATCHv5 16/16] at91-sama5d3_xplained.dts: select ov2640

[Hans Verkuil]

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

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-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
 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 c6b07f83578b..6951bd6a11d7 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>;
 						};
 					};
-- 
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

Re: [PATCHv5 00/16] atmel-isi/ov7670/ov2640: convert to standalone drivers

[Sakari Ailus]

Hi Hans,

On Sat, Mar 11, 2017 at 12:23:12PM +0100, Hans Verkuil wrote:
> 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 6 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.

A question on the last two patches --- what's in the commit description
still holds, right? In that case, how about dropping them from the set?
There have been no changes to those patches during review AFAIR.

-- 
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

[PATCHv5 02/16] ov7670: call v4l2_async_register_subdev

[Hans Verkuil]

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

Add v4l2-async support for this driver.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 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

[PATCHv5 03/16] ov7670: fix g/s_parm

[Hans Verkuil]

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

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 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

[PATCHv5 04/16] ov7670: get xclk

[Hans Verkuil]

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

Get the clock for this sensor.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov7670.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 50e4466a2b37..912ff09c6100 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,13 +1589,24 @@ 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) {
+		ret = -EINVAL;
+		goto clk_disable;
+	}
+
 	/* Make sure it's an ov7670 */
 	ret = ov7670_detect(sd);
 	if (ret) {
 		v4l_dbg(1, debug, client,
 			"chip found @ 0x%x (%s) is not an ov7670 chip.\n",
 			client->addr << 1, client->adapter->name);
-		return ret;
+		goto clk_disable;
 	}
 	v4l_info(client, "chip found @ 0x%02x (%s)\n",
 			client->addr << 1, client->adapter->name);
@@ -1656,6 +1669,8 @@ static int ov7670_probe(struct i2c_client *client,
 
 hdl_free:
 	v4l2_ctrl_handler_free(&info->hdl);
+clk_disable:
+	clk_disable_unprepare(info->clk);
 	return ret;
 }
 
@@ -1667,6 +1682,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

[PATCHv5 06/16] atmel-isi: document device tree bindings

[Hans Verkuil]

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

Document the device tree bindings for this hardware.

Mostly copied from the atmel-isc bindings.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../devicetree/bindings/media/atmel-isi.txt        | 96 +++++++++++++---------
 1 file changed, 58 insertions(+), 38 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt b/Documentation/devicetree/bindings/media/atmel-isi.txt
index 251f008f220c..65249bbd5c00 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
-----------------------------------------------
-
-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.
-
-ISI supports a single port node with parallel bus. It should contain one
+Atmel Image Sensor Interface (ISI)
+----------------------------------
+
+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 shall 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>;
+Endpoint node properties
+------------------------
 
-		clocks = <&isi_clk>;
-		clock-names = "isi_clk";
+- bus-width: <8> or <10> (mandatory)
+- hsync-active (default: active high)
+- vsync-active (default: active high)
+- pclk-sample (default: sample on falling edge)
+- remote-endpoint: A phandle to the bus receiver's endpoint node (mandatory).
 
-		pinctrl-names = "default";
-		pinctrl-0 = <&pinctrl_isi>;
-
-		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

Re: [PATCHv5 06/16] atmel-isi: document device tree bindings

[Rob Herring]

On Sat, Mar 11, 2017 at 12:23:18PM +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.

This commit message doesn't really reflect what you are doing and the 
reformatting and fixes really make this a PIA to review.

>
> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> Acked-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
>  .../devicetree/bindings/media/atmel-isi.txt        | 96 +++++++++++++---------
>  1 file changed, 58 insertions(+), 38 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt b/Documentation/devicetree/bindings/media/atmel-isi.txt
> index 251f008f220c..65249bbd5c00 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
> -----------------------------------------------
> -
> -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.
> -
> -ISI supports a single port node with parallel bus. It should contain one
> +Atmel Image Sensor Interface (ISI)
> +----------------------------------
> +
> +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 shall 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>;
> +Endpoint node properties
> +------------------------
>  
> -		clocks = <&isi_clk>;
> -		clock-names = "isi_clk";
> +- bus-width: <8> or <10> (mandatory)
> +- hsync-active (default: active high)
> +- vsync-active (default: active high)
> +- pclk-sample (default: sample on falling edge)
> +- remote-endpoint: A phandle to the bus receiver's endpoint node (mandatory).
>  
> -		pinctrl-names = "default";
> -		pinctrl-0 = <&pinctrl_isi>;
> -
> -		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";

Don't put status in examples.

> +	port {
> +		#address-cells = <1>;
> +		#size-cells = <0>;

These can be dropped.

> +		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>;

reset-gpios?

> +		pwdn-gpios = <&pioE 13 GPIO_ACTIVE_HIGH>;

powerdown-gpios?

> +		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
--
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: [PATCHv5 06/16] atmel-isi: document device tree bindings

[Hans Verkuil]

On 03/20/2017 05:41 PM, Rob Herring wrote:
> On Sat, Mar 11, 2017 at 12:23:18PM +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.
> 
> This commit message doesn't really reflect what you are doing and the 
> reformatting and fixes really make this a PIA to review.

I'll improve the commit log.

> 
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
>> Acked-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>> ---
>>  .../devicetree/bindings/media/atmel-isi.txt        | 96 +++++++++++++---------
>>  1 file changed, 58 insertions(+), 38 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt b/Documentation/devicetree/bindings/media/atmel-isi.txt
>> index 251f008f220c..65249bbd5c00 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
>> -----------------------------------------------
>> -
>> -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.
>> -
>> -ISI supports a single port node with parallel bus. It should contain one
>> +Atmel Image Sensor Interface (ISI)
>> +----------------------------------
>> +
>> +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 shall 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>;
>> +Endpoint node properties
>> +------------------------
>>  
>> -		clocks = <&isi_clk>;
>> -		clock-names = "isi_clk";
>> +- bus-width: <8> or <10> (mandatory)
>> +- hsync-active (default: active high)
>> +- vsync-active (default: active high)
>> +- pclk-sample (default: sample on falling edge)
>> +- remote-endpoint: A phandle to the bus receiver's endpoint node (mandatory).
>>  
>> -		pinctrl-names = "default";
>> -		pinctrl-0 = <&pinctrl_isi>;
>> -
>> -		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";
> 
> Don't put status in examples.

Will remove.

> 
>> +	port {
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
> 
> These can be dropped.

Ack.

> 
>> +		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>;
> 
> reset-gpios?
> 
>> +		pwdn-gpios = <&pioE 13 GPIO_ACTIVE_HIGH>;
> 
> powerdown-gpios?

I can't change this: these two properties have been in use for a long time for the ov2640
driver.

Regards,

	Hans

> 
>> +		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

--
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: [PATCHv5 06/16] atmel-isi: document device tree bindings

[Rob Herring]

On Mon, Mar 20, 2017 at 11:49 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 03/20/2017 05:41 PM, Rob Herring wrote:
>> On Sat, Mar 11, 2017 at 12:23:18PM +0100, Hans Verkuil wrote:
>>> From: Hans Verkuil <hans.verkuil@cisco.com>

>>> +    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>;
>>
>> reset-gpios?
>>
>>> +            pwdn-gpios = <&pioE 13 GPIO_ACTIVE_HIGH>;
>>
>> powerdown-gpios?
>
> I can't change this: these two properties have been in use for a long time for the ov2640
> driver.

NM. I was thinking this was the ov7670 you just added.

Rob

[PATCHv5 07/16] atmel-isi: remove dependency of the soc-camera framework

[Hans Verkuil]

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/platform/soc_camera/Kconfig     |    3 +-
 drivers/media/platform/soc_camera/atmel-isi.c | 1209 +++++++++++++++----------
 2 files changed, 714 insertions(+), 498 deletions(-)

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..a6d60c2e207d 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;
+	size = isi->fmt.fmt.pix.sizeimage;
 
-	if (!*nbuffers || *nbuffers > MAX_BUFFER_NUM)
-		*nbuffers = MAX_BUFFER_NUM;
-
-	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) {
+		dev_err(isi->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)
+		dev_err(isi->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 vb2_queue_init(q);
+	return 0;
 }
 
-static int isi_camera_set_fmt(struct soc_camera_device *icd,
-			      struct v4l2_format *f)
+static struct isi_format *find_format_by_fourcc(struct atmel_isi *isi,
+						 unsigned int fourcc)
 {
-	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
-	const struct soc_camera_format_xlate *xlate;
-	struct v4l2_pix_format *pix = &f->fmt.pix;
+	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 NULL;
+}
+
+static int isi_try_fmt(struct atmel_isi *isi, struct v4l2_format *f,
+			struct isi_format **current_fmt)
+{
+	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);
 
-	return vb2_poll(&icd->vb2_vidq, file, pt);
+	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	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,335 @@ 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) {
+		dev_err(isi->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_dbg(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_dbg(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;
+	int ret = 0;
+
+	while (1) {
+		ep = of_graph_get_next_endpoint(node, ep);
+		if (!ep)
+			break;
+
+		if (ret) {
+			dev_err(isi->dev,
+				"More then one subdev found in graph\n");
+			of_node_put(ep);
+			return -EINVAL;
+		}
+
+		remote = of_graph_get_remote_port_parent(ep);
+		if (!remote) {
+			of_node_put(ep);
+			return -EINVAL;
+		}
+
+		/* Remote node to connect */
+		isi->entity.node = remote;
+		isi->entity.asd.match_type = V4L2_ASYNC_MATCH_OF;
+		isi->entity.asd.match.of.node = remote;
+		ret++;
+	}
+
+	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;
+	}
+
+	/* 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 +1209,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 +1299,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 +1349,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 +1367,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@atmel.com>");
 MODULE_DESCRIPTION("The V4L2 driver for Atmel Linux");
-- 
2.11.0

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

[Sakari Ailus]

Hi Hans,

On Sat, Mar 11, 2017 at 12:23:19PM +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/platform/soc_camera/Kconfig     |    3 +-
>  drivers/media/platform/soc_camera/atmel-isi.c | 1209 +++++++++++++++----------
>  2 files changed, 714 insertions(+), 498 deletions(-)
> 
> 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..a6d60c2e207d 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c

...

> +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;
> +	}
> +
> +	/* 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;

You can replace this by

	return 0;

And remove the if () below.

> +
> +done:
> +	if (ret < 0) {
> +		v4l2_async_notifier_unregister(&isi->notifier);
> +		of_node_put(isi->entity.node);
> +	}
> +
> +	return ret;
> +}
> +

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

-- 
Regards,

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

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

[Guennadi Liakhovetski]

Hi Hans,

Thanks for the patch. Why hasn't this patch been CCed to Josh Wu? Is he 
still at Atmel? Adding to CC to check.

A general comment: this patch doesn't only remove soc-camera dependencies, 
it also changes the code and the behaviour. I would prefer to break that 
down in multiple patches, or remove this driver completely and add a new 
one. I'll provide some comments, but of course I cannot make an extensive 
review of a 1200-line of change patch without knowing the hardware and the 
set ups well enough.

On Sat, 11 Mar 2017, 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/platform/soc_camera/Kconfig     |    3 +-
>  drivers/media/platform/soc_camera/atmel-isi.c | 1209 +++++++++++++++----------
>  2 files changed, 714 insertions(+), 498 deletions(-)
> 
> 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..a6d60c2e207d 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[] = {

This isn't a const array, you're modifying it during initialisation. Are 
we sure there aren't going to be any SoCs with more than one ISI?

> +	{ V4L2_PIX_FMT_YUYV, MEDIA_BUS_FMT_YUYV8_2X8,
> +	  2, ISI_CFG2_YCC_SWAP_DEFAULT, false },

This struct has 6 elements and is defined almost a 100 lines above this 
initialisation. I'd find using explicit field names easier to read, 
especially since you only initialise 5 out of 6 fields explicitly. You 
also explicitly set the fifth field everywhere to false and leave the 
sixth field implicitly to 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 },

Why are you dropping other conversions to RGB565? Were they wrong?

> +};
>  
> -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;

Superfluous initialisation of cfg2.

> +	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;
> +	size = isi->fmt.fmt.pix.sizeimage;
>  
> -	if (!*nbuffers || *nbuffers > MAX_BUFFER_NUM)
> -		*nbuffers = MAX_BUFFER_NUM;
> -
> -	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) {
> +		dev_err(isi->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);

Isn't this also a change in behaviour? Starting DMA also if no buffers 
have been queued yet?

> +	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)
> +		dev_err(isi->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 vb2_queue_init(q);
> +	return 0;
>  }
>  
> -static int isi_camera_set_fmt(struct soc_camera_device *icd,
> -			      struct v4l2_format *f)
> +static struct isi_format *find_format_by_fourcc(struct atmel_isi *isi,
> +						 unsigned int fourcc)
>  {
> -	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
> -	const struct soc_camera_format_xlate *xlate;
> -	struct v4l2_pix_format *pix = &f->fmt.pix;
> +	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 NULL;
> +}
> +
> +static int isi_try_fmt(struct atmel_isi *isi, struct v4l2_format *f,
> +			struct isi_format **current_fmt)
> +{
> +	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);

As far as I understand the bus configuration selection is now performed 
based on fixed platform data or device tree parameters.

> -		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);
>  
> -	return vb2_poll(&icd->vb2_vidq, file, pt);
> +	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +		return -EINVAL;
> +
> +	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,335 @@ 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];

Superfluous too.

> +
> +	while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
> +	       NULL, &mbus_code)) {
> +		fmt = find_format_by_code(mbus_code.code, &i);

find_format_by_code() assigns i, but it is never used.

> +		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++;
> +	}

Hm, how about

+static int isi_formats_init(struct atmel_isi *isi)
+{
+	struct isi_format *isi_fmts[ARRAY_SIZE(isi_formats)];
+	unsigned int n_fmts = 0, i, j;
...
+	
+	while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
+		NULL, &mbus_code)) {
+		for (i = 0; i < ARRAY_SIZE(isi_formats); i++)
+			if (isi_formats[i].mbus_code == mbus_code.code) {
+				/* Code supported, have we got this fourcc yet? */
+				for (j = 0; j < n_fmts; j++)
+					if (isi_fmts[j]->fourcc == 
+						isi_formats[i].fourcc)
+						/* Already available */
+						break;
+				if (j == n_fmts)
+					/* new */
+					isi_fmts[n_fmts++] = isi_formats + i;
+			}
+		mbus_code.index++;
+	}
+
+	devm_kcalloc(dev, n_fmts, ...);
...

Without calling .enum_mbus_code() multiple times for the same index, 
without .skip, without .support, without overwriting static data...

Thanks
Guennadi
--
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: [PATCHv5 07/16] atmel-isi: remove dependency of the soc-camera framework

[Hans Verkuil]

On 03/12/2017 05:44 PM, Guennadi Liakhovetski wrote:
> Hi Hans,
> 
> Thanks for the patch. Why hasn't this patch been CCed to Josh Wu? Is he 
> still at Atmel? Adding to CC to check.

To the best of my knowledge Josh no longer works for Atmel/Microchip, and Songjun
took over.

> 
> A general comment: this patch doesn't only remove soc-camera dependencies, 
> it also changes the code and the behaviour. I would prefer to break that 
> down in multiple patches, or remove this driver completely and add a new 
> one. I'll provide some comments, but of course I cannot make an extensive 
> review of a 1200-line of change patch without knowing the hardware and the 
> set ups well enough.
> 
> On Sat, 11 Mar 2017, 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/platform/soc_camera/Kconfig     |    3 +-
>>  drivers/media/platform/soc_camera/atmel-isi.c | 1209 +++++++++++++++----------
>>  2 files changed, 714 insertions(+), 498 deletions(-)
>>
>> 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..a6d60c2e207d 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[] = {
> 
> This isn't a const array, you're modifying it during initialisation. Are 
> we sure there aren't going to be any SoCs with more than one ISI?

When that happens, this should be changed at that time. I think it is unlikely
since as I understand it ISI has been replaced by ISC (atmel-isc).

> 
>> +	{ V4L2_PIX_FMT_YUYV, MEDIA_BUS_FMT_YUYV8_2X8,
>> +	  2, ISI_CFG2_YCC_SWAP_DEFAULT, false },
> 
> This struct has 6 elements and is defined almost a 100 lines above this 
> initialisation. I'd find using explicit field names easier to read, 

Moved down and use field names.

> especially since you only initialise 5 out of 6 fields explicitly. You 
> also explicitly set the fifth field everywhere to false and leave the 
> sixth field implicitly to 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 },
> 
> Why are you dropping other conversions to RGB565? Were they wrong?

There was a reason, but I have forgotten it :-(

I'll have to retest the other three YUYV combinations.

> 
>> +};
>>  
>> -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;
> 
> Superfluous initialisation of cfg2.

Fixed.

> 
>> +	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;
>> +	size = isi->fmt.fmt.pix.sizeimage;
>>  
>> -	if (!*nbuffers || *nbuffers > MAX_BUFFER_NUM)
>> -		*nbuffers = MAX_BUFFER_NUM;
>> -
>> -	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) {
>> +		dev_err(isi->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);
> 
> Isn't this also a change in behaviour? Starting DMA also if no buffers 
> have been queued yet?

I now set 'q->min_buffers_needed = 2' so this can never be called with less
than 2 buffers queued. 'min_buffers_needed' didn't exist when this code was
written, but now we can use it.

> 
>> +	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)
>> +		dev_err(isi->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 vb2_queue_init(q);
>> +	return 0;
>>  }
>>  
>> -static int isi_camera_set_fmt(struct soc_camera_device *icd,
>> -			      struct v4l2_format *f)
>> +static struct isi_format *find_format_by_fourcc(struct atmel_isi *isi,
>> +						 unsigned int fourcc)
>>  {
>> -	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
>> -	const struct soc_camera_format_xlate *xlate;
>> -	struct v4l2_pix_format *pix = &f->fmt.pix;
>> +	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 NULL;
>> +}
>> +
>> +static int isi_try_fmt(struct atmel_isi *isi, struct v4l2_format *f,
>> +			struct isi_format **current_fmt)
>> +{
>> +	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);
> 
> As far as I understand the bus configuration selection is now performed 
> based on fixed platform data or device tree parameters.

Correct.

> 
>> -		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);
>>  
>> -	return vb2_poll(&icd->vb2_vidq, file, pt);
>> +	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>> +		return -EINVAL;
>> +
>> +	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,335 @@ 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];
> 
> Superfluous too.

Fixed.

> 
>> +
>> +	while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
>> +	       NULL, &mbus_code)) {
>> +		fmt = find_format_by_code(mbus_code.code, &i);
> 
> find_format_by_code() assigns i, but it is never used.
> 
>> +		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++;
>> +	}
> 
> Hm, how about
> 
> +static int isi_formats_init(struct atmel_isi *isi)
> +{
> +	struct isi_format *isi_fmts[ARRAY_SIZE(isi_formats)];
> +	unsigned int n_fmts = 0, i, j;
> ...
> +	
> +	while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
> +		NULL, &mbus_code)) {
> +		for (i = 0; i < ARRAY_SIZE(isi_formats); i++)
> +			if (isi_formats[i].mbus_code == mbus_code.code) {
> +				/* Code supported, have we got this fourcc yet? */
> +				for (j = 0; j < n_fmts; j++)
> +					if (isi_fmts[j]->fourcc == 
> +						isi_formats[i].fourcc)
> +						/* Already available */
> +						break;
> +				if (j == n_fmts)
> +					/* new */
> +					isi_fmts[n_fmts++] = isi_formats + i;
> +			}
> +		mbus_code.index++;
> +	}
> +
> +	devm_kcalloc(dev, n_fmts, ...);
> ...
> 
> Without calling .enum_mbus_code() multiple times for the same index, 
> without .skip, without .support, without overwriting static data...

Yes, much better. Thanks! This change supercedes some of my comments I made
above since they no longer apply.

Regards,

	Hans

> 
> Thanks
> Guennadi
> 

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

[Nicolas Ferre]

Le 13/03/2017 à 12:43, Hans Verkuil a écrit :
> On 03/12/2017 05:44 PM, Guennadi Liakhovetski wrote:
>> Hi Hans,
>>
>> Thanks for the patch. Why hasn't this patch been CCed to Josh Wu? Is he 
>> still at Atmel? Adding to CC to check.
> 
> To the best of my knowledge Josh no longer works for Atmel/Microchip, and Songjun
> took over.

Yes absolutely, Josh Wu is no longer with us and Songjun and Ludovic
took over the maintenance.
But we have full confidence in experts like you guys and we thank you so
much Hans for having handled this move for atmel-isi.

>> A general comment: this patch doesn't only remove soc-camera dependencies, 
>> it also changes the code and the behaviour. I would prefer to break that 
>> down in multiple patches, or remove this driver completely and add a new 
>> one. I'll provide some comments, but of course I cannot make an extensive 
>> review of a 1200-line of change patch without knowing the hardware and the 
>> set ups well enough.
>>
>> On Sat, 11 Mar 2017, 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/platform/soc_camera/Kconfig     |    3 +-
>>>  drivers/media/platform/soc_camera/atmel-isi.c | 1209 +++++++++++++++----------
>>>  2 files changed, 714 insertions(+), 498 deletions(-)

[..]

>>> +static struct isi_format isi_formats[] = {
>>
>> This isn't a const array, you're modifying it during initialisation. Are 
>> we sure there aren't going to be any SoCs with more than one ISI?
> 
> When that happens, this should be changed at that time. I think it is unlikely
> since as I understand it ISI has been replaced by ISC (atmel-isc).

Yes, ISC has replaced ISI for all Atmel/Microchip MPUs onwards. We may
have several of them, but very likely never more than one ISI.

Regards,
-- 
Nicolas Ferre
--
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

[PATCHv5 08/16] 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>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.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

[PATCHv5 09/16] ov2640: fix colorspace handling

[Hans Verkuil]

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

The colorspace is independent of whether YUV or RGB is sent to the SoC.
Fix this.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/soc_camera/ov2640.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 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)
-- 
2.11.0

Re: [PATCHv5 09/16] ov2640: fix colorspace handling

[Guennadi Liakhovetski]

On Sat, 11 Mar 2017, Hans Verkuil wrote:

> From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> 
> The colorspace is independent of whether YUV or RGB is sent to the SoC.
> Fix this.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

I'm not sure why the first hunk is needed and how it is related :-) But it 
doesn't break anything. I understand, this patch should better go in as a 
part of a series, so

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

Thanks
Guennadi

> ---
>  drivers/media/i2c/soc_camera/ov2640.c | 23 +++++++----------------
>  1 file changed, 7 insertions(+), 16 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)
> -- 
> 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

[PATCHv5 10/16] ov2640: update bindings

[Hans Verkuil]

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

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../devicetree/bindings/media/i2c/ov2640.txt       | 23 +++++++++-------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2640.txt b/Documentation/devicetree/bindings/media/i2c/ov2640.txt
index c429b5bdcaa0..989ce6cb6ac3 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,26 +20,21 @@ 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 {
 				ov2640_0: endpoint {
 					remote-endpoint = <&isi_0>;
-					bus-width = <8>;
 				};
 			};
 		};
-- 
2.11.0

[PATCHv5 12/16] ov2640: use standard clk and enable it.

[Hans Verkuil]

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

Convert v4l2_clk to normal clk and enable the clock.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov2640.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index 83f88efbce69..0445963c5fae 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,14 +1051,11 @@ 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);
@@ -1074,25 +1071,25 @@ 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);
+	clk_disable_unprepare(priv->clk);
 	return ret;
 }
 
@@ -1101,9 +1098,9 @@ 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);
+	clk_disable_unprepare(priv->clk);
 	return 0;
 }
 
-- 
2.11.0

[PATCHv5 14/16] em28xx: drop last soc_camera link

[Hans Verkuil]

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

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@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 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

[PATCHv5 15/16] 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 | 59 ++++++++++++++++++++++++++---
 arch/arm/boot/dts/sama5d3.dtsi              |  4 +-
 2 files changed, 56 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..c6b07f83578b 100644
--- a/arch/arm/boot/dts/at91-sama5d3_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
@@ -65,18 +65,51 @@
 				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>;
+						};
+					};
+				};
+
 				pmic: act8865@5b {
 					compatible = "active-semi,act8865";
 					reg = <0x5b>;
-					status = "disabled";
+					status = "okay";
 
 					regulators {
 						vcc_1v8_reg: DCDC_REG1 {
@@ -130,7 +163,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 +176,7 @@
 			};
 
 			uart0: serial@f0024000 {
-				status = "okay";
+				status = "disabled";
 			};
 
 			mmc1: mmc@f8000000 {
@@ -181,7 +214,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 +233,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

Re: [PATCHv5 00/16] atmel-isi/ov7670/ov2640: convert to standalone drivers

[Sakari Ailus]

On Sat, Mar 11, 2017 at 12:23:12PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This patch series converts the soc-camera atmel-isi to a standalone V4L2
> driver.

Patches 5 and 13:

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

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