Groups > linux.kernel > #1672724 > unrolled thread

[PATCH v1 0/6] Add support of OV9655 camera

Started by	Hugues Fruchet <hugues.fruchet@st.com>
First post	2017-06-22 17:10 +0200
Last post	2017-06-26 12:10 +0200
Articles	20 on this page of 53 — 12 participants

Back to article view | Back to linux.kernel

  [PATCH v1 0/6] Add support of OV9655 camera Hugues Fruchet <hugues.fruchet@st.com> - 2017-06-22 17:10 +0200
    [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module Hugues Fruchet <hugues.fruchet@st.com> - 2017-06-22 17:10 +0200
      Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-23 12:30 +0200
        Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module Andreas Färber <afaerber@suse.de> - 2017-06-23 12:50 +0200
          Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-23 13:10 +0200
            Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2017-06-23 14:00 +0200
              Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-23 17:00 +0200
                Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module Andreas Färber <afaerber@suse.de> - 2017-06-23 17:00 +0200
                  Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-23 17:30 +0200
                    Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module Suman Anna <s-anna@ti.com> - 2017-06-23 20:10 +0200
                      Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-23 21:10 +0200
                        Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module Suman Anna <s-anna@ti.com> - 2017-06-24 00:30 +0200
                          Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-26 08:10 +0200
        Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module Hugues FRUCHET <hugues.fruchet@st.com> - 2017-06-26 12:40 +0200
          Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module Sylwester Nawrocki <snawrocki@kernel.org> - 2017-06-26 22:10 +0200
            Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-27 07:50 +0200
              Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module Sylwester Nawrocki <snawrocki@kernel.org> - 2017-06-28 01:00 +0200
                Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-28 11:20 +0200
                  Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera  module Sylwester Nawrocki <s.nawrocki@samsung.com> - 2017-06-28 13:00 +0200
                    Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-28 13:30 +0200
                      Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module Hugues FRUCHET <hugues.fruchet@st.com> - 2017-06-28 14:30 +0200
        Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module Rob Herring <robh@kernel.org> - 2017-06-26 21:00 +0200
      Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module Rob Herring <robh@kernel.org> - 2017-06-26 21:00 +0200
    [PATCH v1 2/6] [media] ov9650: add device tree support Hugues Fruchet <hugues.fruchet@st.com> - 2017-06-22 17:10 +0200
      Re: [PATCH v1 2/6] [media] ov9650: add device tree support Sakari Ailus <sakari.ailus@iki.fi> - 2017-06-26 18:40 +0200
        Re: [PATCH v1 2/6] [media] ov9650: add device tree support "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-26 19:50 +0200
          Re: [PATCH v1 2/6] [media] ov9650: add device tree support Sakari Ailus <sakari.ailus@iki.fi> - 2017-06-27 07:40 +0200
            Re: [PATCH v1 2/6] [media] ov9650: add device tree support Hugues FRUCHET <hugues.fruchet@st.com> - 2017-06-27 12:20 +0200
    [PATCH v1 4/6] [media] ov9650: use write_array() for resolution sequences Hugues Fruchet <hugues.fruchet@st.com> - 2017-06-22 17:10 +0200
      Re: [PATCH v1 4/6] [media] ov9650: use write_array() for resolution  sequences Sakari Ailus <sakari.ailus@iki.fi> - 2017-06-26 18:40 +0200
        Re: [PATCH v1 4/6] [media] ov9650: use write_array() for resolution  sequences Hugues FRUCHET <hugues.fruchet@st.com> - 2017-06-29 16:10 +0200
    [PATCH v1 3/6] [media] ov9650: select the nearest higher resolution Hugues Fruchet <hugues.fruchet@st.com> - 2017-06-22 17:10 +0200
    [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant Hugues Fruchet <hugues.fruchet@st.com> - 2017-06-22 17:10 +0200
      Re: [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant kbuild test robot <lkp@intel.com> - 2017-06-25 18:10 +0200
      [PATCH] ov9650: fix semicolon.cocci warnings kbuild test robot <lkp@intel.com> - 2017-06-25 18:10 +0200
      Re: [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-26 08:10 +0200
    Re: [PATCH v1 0/6] Add support of OV9655 camera "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-22 17:50 +0200
      Re: [PATCH v1 0/6] Add support of OV9655 camera "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-23 12:30 +0200
        omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655 camera Pavel Machek <pavel@ucw.cz> - 2017-06-25 11:20 +0200
          Re: omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655 camera "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-26 08:10 +0200
            Re: omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655  camera Pavel Machek <pavel@ucw.cz> - 2017-06-26 10:40 +0200
              Re: omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655 camera "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-26 12:00 +0200
                Re: omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655  camera Pavel Machek <pavel@ucw.cz> - 2017-06-26 13:20 +0200
                  Re: omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655 camera "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-27 08:00 +0200
              Re: omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655  camera Hugues FRUCHET <hugues.fruchet@st.com> - 2017-06-26 15:30 +0200
                Re: omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655  camera Hugues FRUCHET <hugues.fruchet@st.com> - 2017-06-27 10:00 +0200
                  Re: [PATCH v1 0/6] Add support of OV9655 camera "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-07-01 23:10 +0200
                    Re: [PATCH v1 0/6] Add support of OV9655 camera Hugues FRUCHET <hugues.fruchet@st.com> - 2017-07-03 10:20 +0200
                      Re: [PATCH v1 0/6] Add support of OV9655 camera "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-07-03 11:20 +0200
                        Re: [PATCH v1 0/6] Add support of OV9655 camera Hugues FRUCHET <hugues.fruchet@st.com> - 2017-07-03 14:10 +0200
                          Re: [PATCH v1 0/6] Add support of OV9655 camera "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-07-03 14:30 +0200
        Re: [PATCH v1 0/6] Add support of OV9655 camera "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-06-26 08:10 +0200
      Re: [PATCH v1 0/6] Add support of OV9655 camera Hugues FRUCHET <hugues.fruchet@st.com> - 2017-06-26 12:10 +0200

Page 2 of 3 — ← Prev page 1 [2] 3 Next page →

#1676619 — Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module

From	Hugues FRUCHET <hugues.fruchet@st.com>
Date	2017-06-28 14:30 +0200
Subject	Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module
Message-ID	<tXkgy-6Kh-13@gated-at.bofh.it>
In reply to	#1676563


On 06/28/2017 01:24 PM, H. Nikolaus Schaller wrote:
> 
>> Am 28.06.2017 um 12:50 schrieb Sylwester Nawrocki <s.nawrocki@samsung.com>:
>>
>> On 06/28/2017 11:12 AM, H. Nikolaus Schaller wrote:
>>>> Am 28.06.2017 um 00:57 schrieb Sylwester Nawrocki <snawrocki@kernel.org>:
>>>> On 06/27/2017 07:48 AM, H. Nikolaus Schaller wrote:
>>>>>> Am 26.06.2017 um 22:04 schrieb Sylwester Nawrocki <snawrocki@kernel.org>:
>>>>>> On 06/26/2017 12:35 PM, Hugues FRUCHET wrote:
>>>>>>>> What I am missing to support the GTA04 camera is the control of the optional "vana-supply".
>>>>>>>> So the driver does not power up the camera module when needed and therefore probing fails.
>>>>>>>>
>>>>>>>>     - vana-supply: a regulator to power up the camera module.
>>>>>>>>
>>>>>>>> Driver code is not complex to add:
>>>>>>
>>>>>>> Yes, I saw it in your code, but as I don't have any programmable power
>>>>>>> supply on my setup, I have not pushed this commit.
>>>>>>
>>>>>> Since you are about to add voltage supplies to the DT binding I'd suggest
>>>>>> to include all three voltage supplies of the sensor chip. Looking at the OV9650
>>>>>> and the OV9655 datasheet there are following names used for the voltage supply
>>>>>> pins:
>>>>>>
>>>>>> AVDD - Analog power supply,
>>>>>> DVDD - Power supply for digital core logic,
>>>>>> DOVDD - Digital power supply for I/O.
>>>>>
>>>>> The latter two are usually not independently switchable from the SoC power
>>>>> the module is connected to.
>>>>>
>>>>> And sometimes DVDD and DOVDD are connected together.
>>>>>
>>>>> So the driver can't make much use of knowing or requesting them because the
>>>>> 1.8V supply is always active, even during suspend.
>>>>>
>>>>>>
>>>>>> I doubt the sensor can work without any of these voltage supplies, thus
>>>>>> regulator_get_optional() should not be used. I would just use the regulator
>>>>>> bulk API to handle all three power supplies.
>>>>>
>>>>> The digital part works with AVDD turned off. So the LDO supplying AVDD should
>>>>> be switchable to save power (&vaux3 on the GTA04 device).>
>>>>> But not all designs can switch it off. Hence the idea to define it as an
>>>>> /optional/ regulator. If it is not defined by DT, the driver simply assumes
>>>>> it is always powered on.
>>>>
>>>> I didn't say we can't define regulator supply properties as optional in the DT
>>>> binding.  If we define them as such and any of these *-supply properties is
>>>> missing in DT with regulator_get() the regulator core will use dummy regulator
>>>> for that particular voltage supply.  While with regulator_get_optional()
>>>> -ENODEV is returned when the regulator cannot be found.
>>>
>>> Ah, ok. I see.
>>>
>>> I had thought that it is the right thing to do like devm_gpiod_get_optional().
>>>
>>> That one it is described as:
>>>
>>> "* This is equivalent to gpiod_get(), except that when no GPIO was assigned to
>>>   * the requested function it will return NULL. This is convenient for drivers
>>>   * that need to handle optional GPIOs."
>>>
>>> Seems to be inconsistent definition of what "optional" means.
>>
>> Indeed, this commit explains it further:
>>
>> commit de1dd9fd2156874b45803299b3b27e65d5defdd9
>> regulator: core: Provide hints to the core about optional supplies
>>
>>> So we indeed should use devm_regulator_get() in this case. Thanks for > pointing out!
>>
>>>>> So in summary we only need AVDD switched for the GTA04 - but it does not
>>>>> matter if the others are optional properties. We would not use them.
>>>>>
>>>>> It does matter if they are mandatory because it adds DT complexity (size
>>>>> and processing) without added function.
>>>>
>>>> We should not be defining DT binding only with selected use cases/board
>>>> designs in mind.  IMO all three voltage supplies should be listed in the
>>>> binding, presumably all can be made optional, with an assumption that when
>>>> the property is missing selected pin is hooked up to a fixed regulator.
>>>
>>> Ok, then it should just be defined in the bindings but not used by
>>> the driver?
>>
>> Yes, I think so. So we have a possibly complete binding right from the
>> beginning. I someone needs handling other supplies than AVDD they could
>> update the driver in future.
> 
> Fine! I have sent some patches to Hughues so that he can integrate it in
> his next version of the patch series.
> 
> BR and thanks,
> Nikolaus
> 

OK got it, I'll push in v2.

[toc] | [prev] | [next] | [standalone]

#1675017 — Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module

From	Rob Herring <robh@kernel.org>
Date	2017-06-26 21:00 +0200
Subject	Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module
Message-ID	<tWHoS-5Yd-27@gated-at.bofh.it>
In reply to	#1673461

On Fri, Jun 23, 2017 at 12:25:33PM +0200, H. Nikolaus Schaller wrote:
> Hi Hugues,
> 
> > Am 22.06.2017 um 17:05 schrieb Hugues Fruchet <hugues.fruchet@st.com>:
> > 
> > From: "H. Nikolaus Schaller" <hns@goldelico.com>
> > 
> > This adds documentation of device tree bindings
> > for the OV965X family camera sensor module.
> > 
> > Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> > Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
> > ---
> > .../devicetree/bindings/media/i2c/ov965x.txt       | 37 ++++++++++++++++++++++
> > 1 file changed, 37 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/i2c/ov965x.txt

[...]

> > +Optional Properties:
> > +- resetb-gpios: reference to the GPIO connected to the resetb pin, if any.
> > +- pwdn-gpios: reference to the GPIO connected to the pwdn pin, if any.
> 
> Here I wonder why you did split that up into two gpios. Each "*-gpios" can have
> multiple entries and if one is not used, a 0 can be specified to make it being ignored.
> 
> But it is up to DT maintainers what they prefer: separate single gpios or a single gpio array.

I think that is pretty clear if you survey a number of bindings (hint: 
it's the former).

Rob

[toc] | [prev] | [next] | [standalone]

#1675013 — Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module

From	Rob Herring <robh@kernel.org>
Date	2017-06-26 21:00 +0200
Subject	Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module
Message-ID	<tWHoR-5Yd-17@gated-at.bofh.it>
In reply to	#1672725

On Thu, Jun 22, 2017 at 05:05:37PM +0200, Hugues Fruchet wrote:
> From: "H. Nikolaus Schaller" <hns@goldelico.com>
> 
> This adds documentation of device tree bindings
> for the OV965X family camera sensor module.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
> ---
>  .../devicetree/bindings/media/i2c/ov965x.txt       | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov965x.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov965x.txt b/Documentation/devicetree/bindings/media/i2c/ov965x.txt
> new file mode 100644
> index 0000000..0e0de1f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov965x.txt
> @@ -0,0 +1,37 @@
> +* Omnivision OV9650/9652/9655 CMOS sensor
> +
> +The Omnivision OV965x sensor support multiple resolutions output, such as
> +CIF, SVGA, UXGA. It also can support YUV422/420, RGB565/555 or raw RGB
> +output format.
> +
> +Required Properties:
> +- compatible: should be one of
> +	"ovti,ov9650"
> +	"ovti,ov9652"
> +	"ovti,ov9655"
> +- clocks: reference to the mclk input clock.
> +
> +Optional Properties:
> +- resetb-gpios: reference to the GPIO connected to the resetb pin, if any.

reset-gpios and state it is active low.

> +- pwdn-gpios: reference to the GPIO connected to the pwdn pin, if any.

powerdown-gpios and state it is active ???.

Those are semi-standard names.

With that,

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

[toc] | [prev] | [next] | [standalone]

#1672728 — [PATCH v1 2/6] [media] ov9650: add device tree support

From	Hugues Fruchet <hugues.fruchet@st.com>
Date	2017-06-22 17:10 +0200
Subject	[PATCH v1 2/6] [media] ov9650: add device tree support
Message-ID	<tVbU6-6qX-23@gated-at.bofh.it>
In reply to	#1672724

Allows use of device tree configuration data.
If no device tree data is there, configuration is taken from platform data.
In order to keep GPIOs configuration compatible between both way of doing,
GPIOs are switched to descriptor-based interface.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 drivers/media/i2c/Kconfig  |  2 +-
 drivers/media/i2c/ov9650.c | 81 ++++++++++++++++++++++++++++++++++------------
 2 files changed, 61 insertions(+), 22 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index c380e24..efea14d 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -595,7 +595,7 @@ config VIDEO_OV7670
 
 config VIDEO_OV9650
 	tristate "OmniVision OV9650/OV9652 sensor support"
-	depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+	depends on GPIOLIB && I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
 	---help---
 	  This is a V4L2 sensor-level driver for the Omnivision
 	  OV9650 and OV9652 camera sensors.
diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index 2de2fbb..8340a45 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -11,12 +11,14 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/gpio.h>
 #include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/media.h>
 #include <linux/module.h>
+#include <linux/of_gpio.h>
 #include <linux/ratelimit.h>
 #include <linux/slab.h>
 #include <linux/string.h>
@@ -249,9 +251,10 @@ struct ov965x {
 	struct v4l2_subdev sd;
 	struct media_pad pad;
 	enum v4l2_mbus_type bus_type;
-	int gpios[NUM_GPIOS];
+	struct gpio_desc *gpios[NUM_GPIOS];
 	/* External master clock frequency */
 	unsigned long mclk_frequency;
+	struct clk *clk;
 
 	/* Protects the struct fields below */
 	struct mutex lock;
@@ -511,10 +514,10 @@ static int ov965x_set_color_matrix(struct ov965x *ov965x)
 	return 0;
 }
 
-static void ov965x_gpio_set(int gpio, int val)
+static void ov965x_gpio_set(struct gpio_desc *gpio, int val)
 {
-	if (gpio_is_valid(gpio))
-		gpio_set_value(gpio, val);
+	if (gpio)
+		gpiod_set_value_cansleep(gpio, val);
 }
 
 static void __ov965x_set_power(struct ov965x *ov965x, int on)
@@ -1406,24 +1409,28 @@ static int ov965x_configure_gpios(struct ov965x *ov965x,
 				  const struct ov9650_platform_data *pdata)
 {
 	int ret, i;
+	int gpios[NUM_GPIOS];
 
-	ov965x->gpios[GPIO_PWDN] = pdata->gpio_pwdn;
-	ov965x->gpios[GPIO_RST]  = pdata->gpio_reset;
+	gpios[GPIO_PWDN] = pdata->gpio_pwdn;
+	gpios[GPIO_RST]  = pdata->gpio_reset;
 
-	for (i = 0; i < ARRAY_SIZE(ov965x->gpios); i++) {
-		int gpio = ov965x->gpios[i];
+	for (i = 0; i < ARRAY_SIZE(gpios); i++) {
+		int gpio = gpios[i];
 
 		if (!gpio_is_valid(gpio))
 			continue;
 		ret = devm_gpio_request_one(&ov965x->client->dev, gpio,
-					    GPIOF_OUT_INIT_HIGH, "OV965X");
-		if (ret < 0)
+					    GPIOF_OUT_INIT_HIGH, DRIVER_NAME);
+		if (ret < 0) {
+			dev_err(&ov965x->client->dev,
+				"Failed to request gpio%d (%d)\n", gpio, ret);
 			return ret;
+		}
 		v4l2_dbg(1, debug, &ov965x->sd, "set gpio %d to 1\n", gpio);
 
 		gpio_set_value(gpio, 1);
 		gpio_export(gpio, 0);
-		ov965x->gpios[i] = gpio;
+		ov965x->gpios[i] = gpio_to_desc(gpio);
 	}
 
 	return 0;
@@ -1469,14 +1476,10 @@ static int ov965x_probe(struct i2c_client *client,
 	struct v4l2_subdev *sd;
 	struct ov965x *ov965x;
 	int ret;
+	struct device_node *np = client->dev.of_node;
 
-	if (pdata == NULL) {
-		dev_err(&client->dev, "platform data not specified\n");
-		return -EINVAL;
-	}
-
-	if (pdata->mclk_frequency == 0) {
-		dev_err(&client->dev, "MCLK frequency not specified\n");
+	if (!pdata && !np) {
+		dev_err(&client->dev, "Platform data or device tree data must be provided\n");
 		return -EINVAL;
 	}
 
@@ -1486,7 +1489,36 @@ static int ov965x_probe(struct i2c_client *client,
 
 	mutex_init(&ov965x->lock);
 	ov965x->client = client;
-	ov965x->mclk_frequency = pdata->mclk_frequency;
+	mutex_init(&ov965x->lock);
+
+	if (np) {
+		/* Device tree */
+		ov965x->gpios[GPIO_RST] =
+			devm_gpiod_get_optional(&client->dev, "resetb",
+						GPIOD_OUT_LOW);
+		ov965x->gpios[GPIO_PWDN] =
+			devm_gpiod_get_optional(&client->dev, "pwdn",
+						GPIOD_OUT_HIGH);
+
+		ov965x->clk = devm_clk_get(&client->dev, NULL);
+		if (IS_ERR(ov965x->clk)) {
+			dev_err(&client->dev, "Could not get clock\n");
+			return PTR_ERR(ov965x->clk);
+		}
+		ov965x->mclk_frequency = clk_get_rate(ov965x->clk);
+	} else {
+		/* Platform data */
+		ret = ov965x_configure_gpios(ov965x, pdata);
+		if (ret < 0)
+			return ret;
+
+		if (pdata->mclk_frequency == 0) {
+			dev_err(&client->dev, "MCLK frequency is mandatory\n");
+			return -EINVAL;
+		}
+		ov965x->mclk_frequency = pdata->mclk_frequency;
+	}
+
 
 	sd = &ov965x->sd;
 	v4l2_i2c_subdev_init(sd, client, &ov965x_subdev_ops);
@@ -1545,15 +1577,22 @@ static int ov965x_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id ov965x_id[] = {
-	{ "OV9650", 0 },
-	{ "OV9652", 0 },
+	{ "OV9650", 0x9650 },
+	{ "OV9652", 0x9652 },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(i2c, ov965x_id);
 
+static const struct of_device_id ov965x_of_match[] = {
+	{ .compatible = "ovti,ov9650", .data = (void *)0x9650 },
+	{ .compatible = "ovti,ov9652", .data = (void *)0x9652 },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ov965x_of_match);
 static struct i2c_driver ov965x_i2c_driver = {
 	.driver = {
 		.name	= DRIVER_NAME,
+		.of_match_table = of_match_ptr(ov965x_of_match),
 	},
 	.probe		= ov965x_probe,
 	.remove		= ov965x_remove,
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1674927 — Re: [PATCH v1 2/6] [media] ov9650: add device tree support

From	Sakari Ailus <sakari.ailus@iki.fi>
Date	2017-06-26 18:40 +0200
Subject	Re: [PATCH v1 2/6] [media] ov9650: add device tree support
Message-ID	<tWFdn-4I2-1@gated-at.bofh.it>
In reply to	#1672728

Hi Hugues,

On Thu, Jun 22, 2017 at 05:05:38PM +0200, Hugues Fruchet wrote:
> @@ -1545,15 +1577,22 @@ static int ov965x_remove(struct i2c_client *client)
>  }
>  
>  static const struct i2c_device_id ov965x_id[] = {
> -	{ "OV9650", 0 },
> -	{ "OV9652", 0 },
> +	{ "OV9650", 0x9650 },
> +	{ "OV9652", 0x9652 },

This change does not appear to match with the patch description nor it the
information is used. How about not changing it, unless there's a reason to?
The same for the data field of the of_device_id array below.

>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(i2c, ov965x_id);
>  
> +static const struct of_device_id ov965x_of_match[] = {
> +	{ .compatible = "ovti,ov9650", .data = (void *)0x9650 },
> +	{ .compatible = "ovti,ov9652", .data = (void *)0x9652 },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ov965x_of_match);
>  static struct i2c_driver ov965x_i2c_driver = {
>  	.driver = {
>  		.name	= DRIVER_NAME,
> +		.of_match_table = of_match_ptr(ov965x_of_match),
>  	},
>  	.probe		= ov965x_probe,
>  	.remove		= ov965x_remove,

-- 
Regards,

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

[toc] | [prev] | [next] | [standalone]

#1674976 — Re: [PATCH v1 2/6] [media] ov9650: add device tree support

From	"H. Nikolaus Schaller" <hns@goldelico.com>
Date	2017-06-26 19:50 +0200
Subject	Re: [PATCH v1 2/6] [media] ov9650: add device tree support
Message-ID	<tWGj8-5mb-13@gated-at.bofh.it>
In reply to	#1674927

Hi,

> Am 26.06.2017 um 18:31 schrieb Sakari Ailus <sakari.ailus@iki.fi>:
> 
> Hi Hugues,
> 
> On Thu, Jun 22, 2017 at 05:05:38PM +0200, Hugues Fruchet wrote:
>> @@ -1545,15 +1577,22 @@ static int ov965x_remove(struct i2c_client *client)
>> }
>> 
>> static const struct i2c_device_id ov965x_id[] = {
>> -	{ "OV9650", 0 },
>> -	{ "OV9652", 0 },
>> +	{ "OV9650", 0x9650 },
>> +	{ "OV9652", 0x9652 },
> 
> This change does not appear to match with the patch description nor it the
> information is used. How about not changing it, unless there's a reason to?
> The same for the data field of the of_device_id array below.

I think it could/should be used to check if the camera chip that is found
by reading the product-id and version registers does match what the device
tree expects and abort probing on a mismatch.

BR,
Nikolaus

[toc] | [prev] | [next] | [standalone]

#1675304 — Re: [PATCH v1 2/6] [media] ov9650: add device tree support

From	Sakari Ailus <sakari.ailus@iki.fi>
Date	2017-06-27 07:40 +0200
Subject	Re: [PATCH v1 2/6] [media] ov9650: add device tree support
Message-ID	<tWRoe-4qg-17@gated-at.bofh.it>
In reply to	#1674976

On Mon, Jun 26, 2017 at 07:46:34PM +0200, H. Nikolaus Schaller wrote:
> Hi,
> 
> > Am 26.06.2017 um 18:31 schrieb Sakari Ailus <sakari.ailus@iki.fi>:
> > 
> > Hi Hugues,
> > 
> > On Thu, Jun 22, 2017 at 05:05:38PM +0200, Hugues Fruchet wrote:
> >> @@ -1545,15 +1577,22 @@ static int ov965x_remove(struct i2c_client *client)
> >> }
> >> 
> >> static const struct i2c_device_id ov965x_id[] = {
> >> -	{ "OV9650", 0 },
> >> -	{ "OV9652", 0 },
> >> +	{ "OV9650", 0x9650 },
> >> +	{ "OV9652", 0x9652 },
> > 
> > This change does not appear to match with the patch description nor it the
> > information is used. How about not changing it, unless there's a reason to?
> > The same for the data field of the of_device_id array below.
> 
> I think it could/should be used to check if the camera chip that is found
> by reading the product-id and version registers does match what the device
> tree expects and abort probing on a mismatch.

Makes sense. But it should be a separate patch, shouldn't it?

You could also put the id to the ops struct, and choose the ops struct that
way. Entirely up to you.

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

[toc] | [prev] | [next] | [standalone]

#1675509 — Re: [PATCH v1 2/6] [media] ov9650: add device tree support

From	Hugues FRUCHET <hugues.fruchet@st.com>
Date	2017-06-27 12:20 +0200
Subject	Re: [PATCH v1 2/6] [media] ov9650: add device tree support
Message-ID	<tWVLe-7w2-61@gated-at.bofh.it>
In reply to	#1675304


On 06/27/2017 07:36 AM, Sakari Ailus wrote:
> On Mon, Jun 26, 2017 at 07:46:34PM +0200, H. Nikolaus Schaller wrote:
>> Hi,
>>
>>> Am 26.06.2017 um 18:31 schrieb Sakari Ailus <sakari.ailus@iki.fi>:
>>>
>>> Hi Hugues,
>>>
>>> On Thu, Jun 22, 2017 at 05:05:38PM +0200, Hugues Fruchet wrote:
>>>> @@ -1545,15 +1577,22 @@ static int ov965x_remove(struct i2c_client *client)
>>>> }
>>>>
>>>> static const struct i2c_device_id ov965x_id[] = {
>>>> -	{ "OV9650", 0 },
>>>> -	{ "OV9652", 0 },
>>>> +	{ "OV9650", 0x9650 },
>>>> +	{ "OV9652", 0x9652 },
>>>
>>> This change does not appear to match with the patch description nor it the
>>> information is used. How about not changing it, unless there's a reason to?
>>> The same for the data field of the of_device_id array below.
>>
>> I think it could/should be used to check if the camera chip that is found
>> by reading the product-id and version registers does match what the device
>> tree expects and abort probing on a mismatch.
> 
> Makes sense. But it should be a separate patch, shouldn't it?
> 
> You could also put the id to the ops struct, and choose the ops struct that
> way. Entirely up to you.
> 

I'll suggest to skip the id check between DT compatible string and real 
device id read from sensor, this is not something I see in other drivers 
currently.
But I would suggest to keep in a separate patch the switch of device id 
names to lower case in order to align with other omnivision cameras and 
not introduce upper/lower case potential bugs in DT later on (as the one 
encountered by Nikolaus):

  [media] ov9650: switch i2c device id to lower case

  static const struct i2c_device_id ov965x_id[] = {
-	{ "OV9650", 0 },
-	{ "OV9652", 0 },
+	{ "ov9650", 0 },
+	{ "ov9652", 0 },


  [media] ov9650: add device tree support

+static const struct of_device_id ov965x_of_match[] = {
+	{ .compatible = "ovti,ov9650", },
+	{ .compatible = "ovti,ov9652", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ov965x_of_match);
+
  static struct i2c_driver ov965x_i2c_driver = {
  	.driver = {
  		.name	= DRIVER_NAME,
+		.of_match_table = of_match_ptr(ov965x_of_match),

[toc] | [prev] | [next] | [standalone]

#1672729 — [PATCH v1 4/6] [media] ov9650: use write_array() for resolution sequences

From	Hugues Fruchet <hugues.fruchet@st.com>
Date	2017-06-22 17:10 +0200
Subject	[PATCH v1 4/6] [media] ov9650: use write_array() for resolution sequences
Message-ID	<tVbU6-6qX-21@gated-at.bofh.it>
In reply to	#1672724

Align resolution sequences on initialization sequence using
i2c_rv structure NULL terminated .This add flexibility
on resolution sequence size.
Document resolution related registers by using corresponding
define instead of hexa address/value.

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 drivers/media/i2c/ov9650.c | 98 ++++++++++++++++++++++++++++++----------------
 1 file changed, 64 insertions(+), 34 deletions(-)

diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index 4311da6..8b283c9 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -227,11 +227,16 @@ struct ov965x_ctrls {
 	u8 update;
 };
 
+struct i2c_rv {
+	u8 addr;
+	u8 value;
+};
+
 struct ov965x_framesize {
 	u16 width;
 	u16 height;
 	u16 max_exp_lines;
-	const u8 *regs;
+	const struct i2c_rv *regs;
 };
 
 struct ov965x_interval {
@@ -280,9 +285,11 @@ struct ov965x {
 	u8 apply_frame_fmt;
 };
 
-struct i2c_rv {
-	u8 addr;
-	u8 value;
+struct ov965x_pixfmt {
+	u32 code;
+	u32 colorspace;
+	/* REG_TSLB value, only bits [3:2] may be set. */
+	u8 tslb_reg;
 };
 
 static const struct i2c_rv ov965x_init_regs[] = {
@@ -342,30 +349,59 @@ struct i2c_rv {
 	{ REG_NULL, 0 }
 };
 
-#define NUM_FMT_REGS 14
-/*
- * COM7,  COM3,  COM4, HSTART, HSTOP, HREF, VSTART, VSTOP, VREF,
- * EXHCH, EXHCL, ADC,  OCOM,   OFON
- */
-static const u8 frame_size_reg_addr[NUM_FMT_REGS] = {
-	0x12, 0x0c, 0x0d, 0x17, 0x18, 0x32, 0x19, 0x1a, 0x03,
-	0x2a, 0x2b, 0x37, 0x38, 0x39,
-};
-
-static const u8 ov965x_sxga_regs[NUM_FMT_REGS] = {
-	0x00, 0x00, 0x00, 0x1e, 0xbe, 0xbf, 0x01, 0x81, 0x12,
-	0x10, 0x34, 0x81, 0x93, 0x51,
+static const struct i2c_rv ov965x_sxga_regs[] = {
+	{ REG_COM7, 0x00 },
+	{ REG_COM3, 0x00 },
+	{ REG_COM4, 0x00 },
+	{ REG_HSTART, 0x1e },
+	{ REG_HSTOP, 0xbe },
+	{ 0x32, 0xbf },
+	{ REG_VSTART, 0x01 },
+	{ REG_VSTOP, 0x81 },
+	{ REG_VREF, 0x12 },
+	{ REG_EXHCH, 0x10 },
+	{ REG_EXHCL, 0x34 },
+	{ REG_ADC, 0x81 },
+	{ REG_ACOM, 0x93 },
+	{ REG_OFON, 0x51 },
+	{ REG_NULL, 0 },
 };
 
-static const u8 ov965x_vga_regs[NUM_FMT_REGS] = {
-	0x40, 0x04, 0x80, 0x26, 0xc6, 0xed, 0x01, 0x3d, 0x00,
-	0x10, 0x40, 0x91, 0x12, 0x43,
+static const struct i2c_rv ov965x_vga_regs[] = {
+	{ REG_COM7, 0x40 },
+	{ REG_COM3, 0x04 },
+	{ REG_COM4, 0x80 },
+	{ REG_HSTART, 0x26 },
+	{ REG_HSTOP, 0xc6 },
+	{ 0x32, 0xed },
+	{ REG_VSTART, 0x01 },
+	{ REG_VSTOP, 0x3d },
+	{ REG_VREF, 0x00 },
+	{ REG_EXHCH, 0x10 },
+	{ REG_EXHCL, 0x40 },
+	{ REG_ADC, 0x91 },
+	{ REG_ACOM, 0x12 },
+	{ REG_OFON, 0x43 },
+	{ REG_NULL, 0 },
 };
 
 /* Determined empirically. */
-static const u8 ov965x_qvga_regs[NUM_FMT_REGS] = {
-	0x10, 0x04, 0x80, 0x25, 0xc5, 0xbf, 0x00, 0x80, 0x12,
-	0x10, 0x40, 0x91, 0x12, 0x43,
+static const struct i2c_rv ov965x_qvga_regs[] = {
+	{ REG_COM7, 0x10 },
+	{ REG_COM3, 0x04 },
+	{ REG_COM4, 0x80 },
+	{ REG_HSTART, 0x25 },
+	{ REG_HSTOP, 0xc5 },
+	{ 0x32, 0xbf },
+	{ REG_VSTART, 0x00 },
+	{ REG_VSTOP, 0x80 },
+	{ REG_VREF, 0x12 },
+	{ REG_EXHCH, 0x10 },
+	{ REG_EXHCL, 0x40 },
+	{ REG_ADC, 0x91 },
+	{ REG_ACOM, 0x12 },
+	{ REG_OFON, 0x43 },
+	{ REG_NULL, 0 },
 };
 
 static const struct ov965x_framesize ov965x_framesizes[] = {
@@ -387,13 +423,6 @@ struct i2c_rv {
 	},
 };
 
-struct ov965x_pixfmt {
-	u32 code;
-	u32 colorspace;
-	/* REG_TSLB value, only bits [3:2] may be set. */
-	u8 tslb_reg;
-};
-
 static const struct ov965x_pixfmt ov965x_formats[] = {
 	{ MEDIA_BUS_FMT_YUYV8_2X8, V4L2_COLORSPACE_JPEG, 0x00},
 	{ MEDIA_BUS_FMT_YVYU8_2X8, V4L2_COLORSPACE_JPEG, 0x04},
@@ -1268,11 +1297,12 @@ static int ov965x_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_pad_config
 
 static int ov965x_set_frame_size(struct ov965x *ov965x)
 {
-	int i, ret = 0;
+	int ret = 0;
+
+	v4l2_dbg(1, debug, ov965x->client, "%s\n", __func__);
 
-	for (i = 0; ret == 0 && i < NUM_FMT_REGS; i++)
-		ret = ov965x_write(ov965x->client, frame_size_reg_addr[i],
-				   ov965x->frame_size->regs[i]);
+	ret = ov965x_write_array(ov965x->client,
+				 ov965x->frame_size->regs);
 	return ret;
 }
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1674930 — Re: [PATCH v1 4/6] [media] ov9650: use write_array() for resolution sequences

From	Sakari Ailus <sakari.ailus@iki.fi>
Date	2017-06-26 18:40 +0200
Subject	Re: [PATCH v1 4/6] [media] ov9650: use write_array() for resolution sequences
Message-ID	<tWFdo-4I2-17@gated-at.bofh.it>
In reply to	#1672729

Hi Hugues,

On Thu, Jun 22, 2017 at 05:05:40PM +0200, Hugues Fruchet wrote:
> Align resolution sequences on initialization sequence using
> i2c_rv structure NULL terminated .This add flexibility
> on resolution sequence size.
> Document resolution related registers by using corresponding
> define instead of hexa address/value.
> 
> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
> ---
>  drivers/media/i2c/ov9650.c | 98 ++++++++++++++++++++++++++++++----------------
>  1 file changed, 64 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
> index 4311da6..8b283c9 100644
> --- a/drivers/media/i2c/ov9650.c
> +++ b/drivers/media/i2c/ov9650.c
> @@ -227,11 +227,16 @@ struct ov965x_ctrls {
>  	u8 update;
>  };
>  
> +struct i2c_rv {
> +	u8 addr;
> +	u8 value;
> +};
> +
>  struct ov965x_framesize {
>  	u16 width;
>  	u16 height;
>  	u16 max_exp_lines;
> -	const u8 *regs;
> +	const struct i2c_rv *regs;
>  };
>  
>  struct ov965x_interval {
> @@ -280,9 +285,11 @@ struct ov965x {
>  	u8 apply_frame_fmt;
>  };
>  
> -struct i2c_rv {
> -	u8 addr;
> -	u8 value;
> +struct ov965x_pixfmt {
> +	u32 code;
> +	u32 colorspace;
> +	/* REG_TSLB value, only bits [3:2] may be set. */
> +	u8 tslb_reg;
>  };
>  
>  static const struct i2c_rv ov965x_init_regs[] = {
> @@ -342,30 +349,59 @@ struct i2c_rv {
>  	{ REG_NULL, 0 }
>  };
>  
> -#define NUM_FMT_REGS 14
> -/*
> - * COM7,  COM3,  COM4, HSTART, HSTOP, HREF, VSTART, VSTOP, VREF,
> - * EXHCH, EXHCL, ADC,  OCOM,   OFON
> - */
> -static const u8 frame_size_reg_addr[NUM_FMT_REGS] = {
> -	0x12, 0x0c, 0x0d, 0x17, 0x18, 0x32, 0x19, 0x1a, 0x03,
> -	0x2a, 0x2b, 0x37, 0x38, 0x39,
> -};
> -
> -static const u8 ov965x_sxga_regs[NUM_FMT_REGS] = {
> -	0x00, 0x00, 0x00, 0x1e, 0xbe, 0xbf, 0x01, 0x81, 0x12,
> -	0x10, 0x34, 0x81, 0x93, 0x51,
> +static const struct i2c_rv ov965x_sxga_regs[] = {
> +	{ REG_COM7, 0x00 },
> +	{ REG_COM3, 0x00 },
> +	{ REG_COM4, 0x00 },
> +	{ REG_HSTART, 0x1e },
> +	{ REG_HSTOP, 0xbe },
> +	{ 0x32, 0xbf },
> +	{ REG_VSTART, 0x01 },
> +	{ REG_VSTOP, 0x81 },
> +	{ REG_VREF, 0x12 },
> +	{ REG_EXHCH, 0x10 },
> +	{ REG_EXHCL, 0x34 },
> +	{ REG_ADC, 0x81 },
> +	{ REG_ACOM, 0x93 },
> +	{ REG_OFON, 0x51 },
> +	{ REG_NULL, 0 },
>  };
>  
> -static const u8 ov965x_vga_regs[NUM_FMT_REGS] = {
> -	0x40, 0x04, 0x80, 0x26, 0xc6, 0xed, 0x01, 0x3d, 0x00,
> -	0x10, 0x40, 0x91, 0x12, 0x43,
> +static const struct i2c_rv ov965x_vga_regs[] = {
> +	{ REG_COM7, 0x40 },
> +	{ REG_COM3, 0x04 },
> +	{ REG_COM4, 0x80 },
> +	{ REG_HSTART, 0x26 },
> +	{ REG_HSTOP, 0xc6 },
> +	{ 0x32, 0xed },
> +	{ REG_VSTART, 0x01 },
> +	{ REG_VSTOP, 0x3d },
> +	{ REG_VREF, 0x00 },
> +	{ REG_EXHCH, 0x10 },
> +	{ REG_EXHCL, 0x40 },
> +	{ REG_ADC, 0x91 },
> +	{ REG_ACOM, 0x12 },
> +	{ REG_OFON, 0x43 },
> +	{ REG_NULL, 0 },
>  };
>  
>  /* Determined empirically. */
> -static const u8 ov965x_qvga_regs[NUM_FMT_REGS] = {
> -	0x10, 0x04, 0x80, 0x25, 0xc5, 0xbf, 0x00, 0x80, 0x12,
> -	0x10, 0x40, 0x91, 0x12, 0x43,
> +static const struct i2c_rv ov965x_qvga_regs[] = {
> +	{ REG_COM7, 0x10 },
> +	{ REG_COM3, 0x04 },
> +	{ REG_COM4, 0x80 },
> +	{ REG_HSTART, 0x25 },
> +	{ REG_HSTOP, 0xc5 },
> +	{ 0x32, 0xbf },
> +	{ REG_VSTART, 0x00 },
> +	{ REG_VSTOP, 0x80 },
> +	{ REG_VREF, 0x12 },
> +	{ REG_EXHCH, 0x10 },
> +	{ REG_EXHCL, 0x40 },
> +	{ REG_ADC, 0x91 },
> +	{ REG_ACOM, 0x12 },
> +	{ REG_OFON, 0x43 },
> +	{ REG_NULL, 0 },
>  };
>  
>  static const struct ov965x_framesize ov965x_framesizes[] = {
> @@ -387,13 +423,6 @@ struct i2c_rv {
>  	},
>  };
>  
> -struct ov965x_pixfmt {
> -	u32 code;
> -	u32 colorspace;
> -	/* REG_TSLB value, only bits [3:2] may be set. */
> -	u8 tslb_reg;
> -};

Any particular reason for moving struct ov965x_pixfmt definition?

> -
>  static const struct ov965x_pixfmt ov965x_formats[] = {
>  	{ MEDIA_BUS_FMT_YUYV8_2X8, V4L2_COLORSPACE_JPEG, 0x00},
>  	{ MEDIA_BUS_FMT_YVYU8_2X8, V4L2_COLORSPACE_JPEG, 0x04},
> @@ -1268,11 +1297,12 @@ static int ov965x_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_pad_config
>  
>  static int ov965x_set_frame_size(struct ov965x *ov965x)
>  {
> -	int i, ret = 0;
> +	int ret = 0;
> +
> +	v4l2_dbg(1, debug, ov965x->client, "%s\n", __func__);
>  
> -	for (i = 0; ret == 0 && i < NUM_FMT_REGS; i++)
> -		ret = ov965x_write(ov965x->client, frame_size_reg_addr[i],
> -				   ov965x->frame_size->regs[i]);
> +	ret = ov965x_write_array(ov965x->client,
> +				 ov965x->frame_size->regs);
>  	return ret;
>  }
>  

-- 
Regards,

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

[toc] | [prev] | [next] | [standalone]

#1677781 — Re: [PATCH v1 4/6] [media] ov9650: use write_array() for resolution sequences

From	Hugues FRUCHET <hugues.fruchet@st.com>
Date	2017-06-29 16:10 +0200
Subject	Re: [PATCH v1 4/6] [media] ov9650: use write_array() for resolution sequences
Message-ID	<tXIiS-80-29@gated-at.bofh.it>
In reply to	#1674930


On 06/26/2017 06:33 PM, Sakari Ailus wrote:
> Hi Hugues,
> 
> On Thu, Jun 22, 2017 at 05:05:40PM +0200, Hugues Fruchet wrote:
>> Align resolution sequences on initialization sequence using
>> i2c_rv structure NULL terminated .This add flexibility
>> on resolution sequence size.
>> Document resolution related registers by using corresponding
>> define instead of hexa address/value.
>>
>> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
>> ---
>>   drivers/media/i2c/ov9650.c | 98 ++++++++++++++++++++++++++++++----------------
>>   1 file changed, 64 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
>> index 4311da6..8b283c9 100644
>> --- a/drivers/media/i2c/ov9650.c
>> +++ b/drivers/media/i2c/ov9650.c
>> @@ -227,11 +227,16 @@ struct ov965x_ctrls {
>>   	u8 update;
>>   };
>>   
>> +struct i2c_rv {
>> +	u8 addr;
>> +	u8 value;
>> +};
>> +
>>   struct ov965x_framesize {
>>   	u16 width;
>>   	u16 height;
>>   	u16 max_exp_lines;
>> -	const u8 *regs;
>> +	const struct i2c_rv *regs;
>>   };
>>   
>>   struct ov965x_interval {
>> @@ -280,9 +285,11 @@ struct ov965x {
>>   	u8 apply_frame_fmt;
>>   };
>>   
>> -struct i2c_rv {
>> -	u8 addr;
>> -	u8 value;
>> +struct ov965x_pixfmt {
>> +	u32 code;
>> +	u32 colorspace;
>> +	/* REG_TSLB value, only bits [3:2] may be set. */
>> +	u8 tslb_reg;
>>   };
>>   
>>   static const struct i2c_rv ov965x_init_regs[] = {
>> @@ -342,30 +349,59 @@ struct i2c_rv {
>>   	{ REG_NULL, 0 }
>>   };
>>   
>> -#define NUM_FMT_REGS 14
>> -/*
>> - * COM7,  COM3,  COM4, HSTART, HSTOP, HREF, VSTART, VSTOP, VREF,
>> - * EXHCH, EXHCL, ADC,  OCOM,   OFON
>> - */
>> -static const u8 frame_size_reg_addr[NUM_FMT_REGS] = {
>> -	0x12, 0x0c, 0x0d, 0x17, 0x18, 0x32, 0x19, 0x1a, 0x03,
>> -	0x2a, 0x2b, 0x37, 0x38, 0x39,
>> -};
>> -
>> -static const u8 ov965x_sxga_regs[NUM_FMT_REGS] = {
>> -	0x00, 0x00, 0x00, 0x1e, 0xbe, 0xbf, 0x01, 0x81, 0x12,
>> -	0x10, 0x34, 0x81, 0x93, 0x51,
>> +static const struct i2c_rv ov965x_sxga_regs[] = {
>> +	{ REG_COM7, 0x00 },
>> +	{ REG_COM3, 0x00 },
>> +	{ REG_COM4, 0x00 },
>> +	{ REG_HSTART, 0x1e },
>> +	{ REG_HSTOP, 0xbe },
>> +	{ 0x32, 0xbf },
>> +	{ REG_VSTART, 0x01 },
>> +	{ REG_VSTOP, 0x81 },
>> +	{ REG_VREF, 0x12 },
>> +	{ REG_EXHCH, 0x10 },
>> +	{ REG_EXHCL, 0x34 },
>> +	{ REG_ADC, 0x81 },
>> +	{ REG_ACOM, 0x93 },
>> +	{ REG_OFON, 0x51 },
>> +	{ REG_NULL, 0 },
>>   };
>>   
>> -static const u8 ov965x_vga_regs[NUM_FMT_REGS] = {
>> -	0x40, 0x04, 0x80, 0x26, 0xc6, 0xed, 0x01, 0x3d, 0x00,
>> -	0x10, 0x40, 0x91, 0x12, 0x43,
>> +static const struct i2c_rv ov965x_vga_regs[] = {
>> +	{ REG_COM7, 0x40 },
>> +	{ REG_COM3, 0x04 },
>> +	{ REG_COM4, 0x80 },
>> +	{ REG_HSTART, 0x26 },
>> +	{ REG_HSTOP, 0xc6 },
>> +	{ 0x32, 0xed },
>> +	{ REG_VSTART, 0x01 },
>> +	{ REG_VSTOP, 0x3d },
>> +	{ REG_VREF, 0x00 },
>> +	{ REG_EXHCH, 0x10 },
>> +	{ REG_EXHCL, 0x40 },
>> +	{ REG_ADC, 0x91 },
>> +	{ REG_ACOM, 0x12 },
>> +	{ REG_OFON, 0x43 },
>> +	{ REG_NULL, 0 },
>>   };
>>   
>>   /* Determined empirically. */
>> -static const u8 ov965x_qvga_regs[NUM_FMT_REGS] = {
>> -	0x10, 0x04, 0x80, 0x25, 0xc5, 0xbf, 0x00, 0x80, 0x12,
>> -	0x10, 0x40, 0x91, 0x12, 0x43,
>> +static const struct i2c_rv ov965x_qvga_regs[] = {
>> +	{ REG_COM7, 0x10 },
>> +	{ REG_COM3, 0x04 },
>> +	{ REG_COM4, 0x80 },
>> +	{ REG_HSTART, 0x25 },
>> +	{ REG_HSTOP, 0xc5 },
>> +	{ 0x32, 0xbf },
>> +	{ REG_VSTART, 0x00 },
>> +	{ REG_VSTOP, 0x80 },
>> +	{ REG_VREF, 0x12 },
>> +	{ REG_EXHCH, 0x10 },
>> +	{ REG_EXHCL, 0x40 },
>> +	{ REG_ADC, 0x91 },
>> +	{ REG_ACOM, 0x12 },
>> +	{ REG_OFON, 0x43 },
>> +	{ REG_NULL, 0 },
>>   };
>>   
>>   static const struct ov965x_framesize ov965x_framesizes[] = {
>> @@ -387,13 +423,6 @@ struct i2c_rv {
>>   	},
>>   };
>>   
>> -struct ov965x_pixfmt {
>> -	u32 code;
>> -	u32 colorspace;
>> -	/* REG_TSLB value, only bits [3:2] may be set. */
>> -	u8 tslb_reg;
>> -};
> 
> Any particular reason for moving struct ov965x_pixfmt definition?

Not in the right patch, must be in 5/6 [media] ov9650: add multiple 
variant support, I'll fix in v2.

> 
>> -
>>   static const struct ov965x_pixfmt ov965x_formats[] = {
>>   	{ MEDIA_BUS_FMT_YUYV8_2X8, V4L2_COLORSPACE_JPEG, 0x00},
>>   	{ MEDIA_BUS_FMT_YVYU8_2X8, V4L2_COLORSPACE_JPEG, 0x04},
>> @@ -1268,11 +1297,12 @@ static int ov965x_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_pad_config
>>   
>>   static int ov965x_set_frame_size(struct ov965x *ov965x)
>>   {
>> -	int i, ret = 0;
>> +	int ret = 0;
>> +
>> +	v4l2_dbg(1, debug, ov965x->client, "%s\n", __func__);
>>   
>> -	for (i = 0; ret == 0 && i < NUM_FMT_REGS; i++)
>> -		ret = ov965x_write(ov965x->client, frame_size_reg_addr[i],
>> -				   ov965x->frame_size->regs[i]);
>> +	ret = ov965x_write_array(ov965x->client,
>> +				 ov965x->frame_size->regs);
>>   	return ret;
>>   }
>>   
>

[toc] | [prev] | [next] | [standalone]

#1672732 — [PATCH v1 3/6] [media] ov9650: select the nearest higher resolution

From	Hugues Fruchet <hugues.fruchet@st.com>
Date	2017-06-22 17:10 +0200
Subject	[PATCH v1 3/6] [media] ov9650: select the nearest higher resolution
Message-ID	<tVbU7-6qX-37@gated-at.bofh.it>
In reply to	#1672724

Refine the resolution selection algorithm by selecting
only the nearest higher resolution (instead of lower and higher).

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 drivers/media/i2c/ov9650.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index 8340a45..4311da6 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -1196,9 +1196,11 @@ static void __ov965x_try_frame_size(struct v4l2_mbus_framefmt *mf,
 	unsigned int min_err = UINT_MAX;
 
 	while (i--) {
-		int err = abs(fsize->width - mf->width)
-				+ abs(fsize->height - mf->height);
-		if (err < min_err) {
+		int w_err = (fsize->width - mf->width);
+		int h_err = (fsize->height - mf->height);
+		int err = w_err + h_err;
+
+		if ((w_err >= 0) && (h_err >= 0) && (err < min_err)) {
 			min_err = err;
 			match = fsize;
 		}
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1672734 — [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant

From	Hugues Fruchet <hugues.fruchet@st.com>
Date	2017-06-22 17:10 +0200
Subject	[PATCH v1 6/6] [media] ov9650: add support of OV9655 variant
Message-ID	<tVbU7-6qX-35@gated-at.bofh.it>
In reply to	#1672724

Add a first support of OV9655 variant.
Because of register set slightly different from OV9650/9652,
not all of the driver features are supported (controls).
Supported resolutions are limited to VGA, QVGA, QQVGA.
Supported format is limited to RGB565.
Controls are limited to color bar test pattern for test purpose.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 drivers/media/i2c/Kconfig  |   4 +-
 drivers/media/i2c/ov9650.c | 486 ++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 457 insertions(+), 33 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index efea14d..a8f638c 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -594,11 +594,11 @@ config VIDEO_OV7670
 	  controller.
 
 config VIDEO_OV9650
-	tristate "OmniVision OV9650/OV9652 sensor support"
+	tristate "OmniVision OV9650/OV9652/OV9655 sensor support"
 	depends on GPIOLIB && I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
 	---help---
 	  This is a V4L2 sensor-level driver for the Omnivision
-	  OV9650 and OV9652 camera sensors.
+	  OV9650 and OV9652 and OV9655 camera sensors.
 
 config VIDEO_VS6624
 	tristate "ST VS6624 sensor support"
diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index a9d268d..c0819af 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -1,5 +1,5 @@
 /*
- * Omnivision OV9650/OV9652 CMOS Image Sensor driver
+ * Omnivision OV9650/OV9652/OV9655 CMOS Image Sensor driver
  *
  * Copyright (C) 2013, Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
  *
@@ -7,6 +7,15 @@
  * by Vladimir Fonov.
  * Copyright (c) 2010, Vladimir Fonov
  *
+ *
+ * Copyright (C) STMicroelectronics SA 2017
+ * Author: Hugues Fruchet <hugues.fruchet@st.com> for STMicroelectronics.
+ *
+ * OV9655 initial support based on a driver written by H. Nikolaus Schaller:
+ *   http://git.goldelico.com/?p=gta04-kernel.git;a=shortlog;h=refs/heads/work/hns/video/ov9655
+ * OV9655 registers sequence from STM32CubeF7 embedded software, see:
+ *   https://developer.mbed.org/teams/ST/code/BSP_DISCO_F746NG/file/e1d9da7fe856/Drivers/BSP/Components/ov9655/ov9655.c
+ *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
@@ -58,14 +67,21 @@
 #define REG_PID			0x0a	/* Product ID MSB */
 #define REG_VER			0x0b	/* Product ID LSB */
 #define REG_COM3		0x0c
-#define  COM3_SWAP		0x40
+#define  COM3_COLORBAR		0x80
+#define  COM3_RGB565		0x00
+#define  COM3_SWAP		0x40	/* Doesn't work in RGB */
+#define  COM3_RESETB		0x08
 #define  COM3_VARIOPIXEL1	0x04
+#define  OV9655_SINGLEFRAME	0x01
 #define REG_COM4		0x0d	/* Vario Pixels  */
 #define  COM4_VARIOPIXEL2	0x80
+#define  OV9655_TRISTATE		/* seems to have a different function */
 #define REG_COM5		0x0e	/* System clock options */
 #define  COM5_SLAVE_MODE	0x10
-#define  COM5_SYSTEMCLOCK48MHZ	0x80
+#define  COM5_SYSTEMCLOCK48MHZ	0x80	/* not on OV9655 */
+#define  OV9655_EXPOSURESTEP	0x01
 #define REG_COM6		0x0f	/* HREF & ADBLC options */
+#define  COM6_BLC_OPTICAL	0x40	/* Optical black */
 #define REG_AECH		0x10	/* Exposure value, AEC[9:2] */
 #define REG_CLKRC		0x11	/* Clock control */
 #define  CLK_EXT		0x40	/* Use external clock directly */
@@ -74,13 +90,18 @@
 #define  COM7_RESET		0x80
 #define  COM7_FMT_MASK		0x38
 #define  COM7_FMT_VGA		0x40
-#define	 COM7_FMT_CIF		0x20
+#define  COM7_FMT_CIF		0x20
 #define  COM7_FMT_QVGA		0x10
 #define  COM7_FMT_QCIF		0x08
-#define	 COM7_RGB		0x04
-#define	 COM7_YUV		0x00
-#define	 COM7_BAYER		0x01
-#define	 COM7_PBAYER		0x05
+#define  COM7_RGB		0x04
+#define  COM7_YUV		0x00
+#define  COM7_BAYER		0x01
+#define  COM7_PBAYER		0x05
+#define  OV9655_COM7_VGA	0x60
+#define  OV9655_COM7_RAWRGB	0x00	/* different format encoding */
+#define  OV9655_COM7_RAWRGBINT	0x01
+#define  OV9655_COM7_YUV	0x02
+#define  OV9655_COM7_RGB	0x03
 #define REG_COM8		0x13	/* AGC/AEC options */
 #define  COM8_FASTAEC		0x80	/* Enable fast AGC/AEC */
 #define  COM8_AECSTEP		0x40	/* Unlimited AEC step size */
@@ -89,14 +110,23 @@
 #define  COM8_AWB		0x02	/* White balance enable */
 #define  COM8_AEC		0x01	/* Auto exposure enable */
 #define REG_COM9		0x14	/* Gain ceiling */
-#define  COM9_GAIN_CEIL_MASK	0x70	/* */
+#define  COM9_GAIN_CEIL_MASK	0x70
+#define  COM9_GAIN_CEIL_16X	0x30
+#define  OV9655_COM9_EXPTIMING	0x08
+#define  OV9655_COM9_VSYNCDROP	0x04
+#define  OV9655_COM9_AECDROP	0x02
 #define REG_COM10		0x15	/* PCLK, HREF, HSYNC signals polarity */
+#define  OV9655_SLAVE_PIN	0x80	/* SLHS/SLVS instead of RESETB/PWDN */
 #define  COM10_HSYNC		0x40	/* HSYNC instead of HREF */
 #define  COM10_PCLK_HB		0x20	/* Suppress PCLK on horiz blank */
-#define  COM10_HREF_REV		0x08	/* Reverse HREF */
+#define  OV9655_COM10_PCLK_REV		0x10	/* PCLK reverse */
+#define  COM10_HREF_REV	0x08	/* Reverse HREF */
 #define  COM10_VS_LEAD		0x04	/* VSYNC on clock leading edge */
+#define  OV9655_COM10_RESET_OPTION	0x04	/* Reset signal end point */
 #define  COM10_VS_NEG		0x02	/* VSYNC negative */
 #define  COM10_HS_NEG		0x01	/* HSYNC negative */
+#define OV9655_REG16		0x16	/* dummy frame and blanking */
+#define   OV9655_REG16_DUMMY_8	0x20	/* dummy frame when gain > 8 */
 #define REG_HSTART		0x17	/* Horiz start high bits */
 #define REG_HSTOP		0x18	/* Horiz stop high bits */
 #define REG_VSTART		0x19	/* Vert start high bits */
@@ -117,6 +147,7 @@
 #define REG_BBIAS		0x27	/* B channel output bias */
 #define REG_GBBIAS		0x28	/* Gb channel output bias */
 #define REG_GRCOM		0x29	/* Analog BLC & regulator */
+#define OV9655_PREGAIN		0x29
 #define REG_EXHCH		0x2a	/* Dummy pixel insert MSB */
 #define REG_EXHCL		0x2b	/* Dummy pixel insert LSB */
 #define REG_RBIAS		0x2c	/* R channel output bias */
@@ -127,12 +158,30 @@
 #define REG_HSYEN		0x31	/* HSYNC falling edge delay LSB*/
 #define REG_HREF		0x32	/* HREF pieces */
 #define REG_CHLF		0x33	/* reserved */
+#define OV9655_CLKF		0x33	/* Array current control */
+#define OV9655_AREF1		0x34	/* Array reference control */
+#define OV9655_AREF2		0x35	/* Array reference control */
+#define OV9655_AREF3		0x36	/* Array reference control */
 #define REG_ADC			0x37	/* reserved */
+#define OV9655_ADC		0x37	/* ADC Control 1 (Range adjustment) */
 #define REG_ACOM		0x38	/* reserved */
-#define REG_OFON		0x39	/* Power down register */
+#define OV9655_ADC2		0x38	/* ADC Control 2 (Range adjustment) */
+#define REG_OFON		0x39	/* Power down register (ov9650 only) */
 #define  OFON_PWRDN		0x08	/* Power down bit */
+#define OV9655_AREF4		0x39	/* Array reference control */
 #define REG_TSLB		0x3a	/* YUVU format */
+#define  OV9655_PCLKDELAY2NS	0x40
+#define  OV9655_PCLKDELAY4NS	0x80
+#define  OV9655_PCLKDELAY6NS	0xc0
+#define  OV9655_OUTREVERSE	0x20
+#define  OV9655_FIXEDUV	0x10
 #define  TSLB_YUYV_MASK		0x0c	/* UYVY or VYUY - see com13 */
+#define  TSLB_YUYV		0x00
+#define  TSLB_YVYU		0x04
+#define  TSLB_VYUY		0x08
+#define  TSLB_UYVY		0x0c
+#define  OV9655_BANDINGAUTO	0x02
+
 #define REG_COM11		0x3b	/* Night mode, banding filter enable */
 #define  COM11_NIGHT		0x80	/* Night mode enable */
 #define  COM11_NMFR		0x60	/* Two bit NM frame rate */
@@ -142,25 +191,38 @@
 #define  COM12_HREF		0x80	/* HREF always */
 #define REG_COM13		0x3d	/* Gamma selection, Color matrix en. */
 #define  COM13_GAMMA		0x80	/* Gamma enable */
-#define	 COM13_UVSAT		0x40	/* UV saturation auto adjustment */
+#define  COM13_UVSAT		0x40	/* UV saturation auto adjustment */
+#define  COM13_Y_DELAY		0x08	/* Delay Y channel */
 #define  COM13_UVSWAP		0x01	/* V before U - w/TSLB */
 #define REG_COM14		0x3e	/* Edge enhancement options */
 #define  COM14_EDGE_EN		0x02
 #define  COM14_EEF_X2		0x01
+#define OV9655_REG_COM14	0x3e	/* pixel correction/zoom ON/OFF sel. */
+#define  OV9655_COM14_BLACK_PIX	0x08	/* Black pixel correction */
+#define  OV9655_COM14_WHITE_PIX	0x04	/* White pixel correction */
+#define  OV9655_COM14_ZOOM	0x02	/* Zoom function ON */
 #define REG_EDGE		0x3f	/* Edge enhancement factor */
 #define  EDGE_FACTOR_MASK	0x0f
 #define REG_COM15		0x40	/* Output range, RGB 555/565 */
 #define  COM15_R10F0		0x00	/* Data range 10 to F0 */
-#define	 COM15_R01FE		0x80	/* 01 to FE */
+#define  COM15_R01FE		0x80	/* 01 to FE */
 #define  COM15_R00FF		0xc0	/* 00 to FF */
 #define  COM15_RGB565		0x10	/* RGB565 output */
 #define  COM15_RGB555		0x30	/* RGB555 output */
 #define  COM15_SWAPRB		0x04	/* Swap R&B */
 #define REG_COM16		0x41	/* Color matrix coeff options */
 #define REG_COM17		0x42	/* Single frame out, banding filter */
+#define OV9655_REG_COM17	0x42	/* Denoise, edge, auto gain, ... */
+#define   OV9655_COM17_EDGE_AUTO	0x40	/* Edge auto */
+#define   OV9655_COM17_DENOISE_AUTO	0x80	/* Denoise auto */
+#define OV9655_REG_RSVD(__n)	(0x43 + (__n) - 1) /* reserved but used... */
 /* n = 1...9, 0x4f..0x57 */
-#define	REG_MTX(__n)		(0x4f + (__n) - 1)
+#define REG_MTX(__n)		(0x4f + (__n) - 1)
 #define REG_MTXS		0x58
+#define REG_AWBOP(__n)		(0x59 + (__n) - 1) /* AWB control options */
+#define REG_BLMT		0x5F	/* AWB Blue Component Gain Limit */
+#define REG_RLMT		0x60	/* AWB Red Component Gain Limit */
+#define REG_GLMT		0x61	/* AWB Green Component Gain Limit */
 /* Lens Correction Option 1...5, __n = 0...5 */
 #define REG_LCC(__n)		(0x62 + (__n) - 1)
 #define  LCC5_LCC_ENABLE	0x01	/* LCC5, enable lens correction */
@@ -170,10 +232,26 @@
 #define REG_HV			0x69	/* Manual banding filter MSB */
 #define REG_MBD			0x6a	/* Manual banding filter value */
 #define REG_DBLV		0x6b	/* reserved */
+#define OV9655_REG_DBLV		0x6b	/* PLL, DVDD regu bypass, bandgap */
+#define  OV9655_DBLV_BANDGAP	0x0a	/* default value */
+#define  OV9655_DBLV_LDO_BYPASS	0x10
+#define  OV9655_DBLV_PLL_BYPASS	0x00
+#define  OV9655_DBLV_PLL_4X	0x40
+#define  OV9655_DBLV_PLL_6X	0x80
+#define  OV9655_DBLV_PLL_8X	0xc0
 #define REG_GSP			0x6c	/* Gamma curve */
 #define  GSP_LEN		15
+#define OV9655_REG_DNSTH	0x70	/* De-noise Function Threshold Adj. */
+#define OV9655_REG_POIDX	0x72	/* Pixel output index */
+#define OV9655_REG_PCKDV	0x73	/* Pixel Clock Output Selection */
+#define OV9655_REG_XINDX	0x74	/* Horizontal Scaling Down Coeff. */
+#define OV9655_REG_YINDX	0x75	/* Vertical Scaling Down Coeff. */
+#define OV9655_REG_SLOP		0x7A	/* Gamma Curve Highest Segment Slope */
+#define OV9655_REG_GAM(__n)	(0x7B + (__n) - 1)	/* Gamma curve */
 #define REG_GST			0x7c	/* Gamma curve */
 #define  GST_LEN		15
+#define OV9655_REG_COM18	0x8b	/* Zoom mode in VGA */
+#define OV9655_REG_COM19	0x8c	/* UV adjustment */
 #define REG_COM21		0x8b
 #define REG_COM22		0x8c	/* Edge enhancement, denoising */
 #define  COM22_WHTPCOR		0x02	/* White pixel correction enable */
@@ -181,6 +259,8 @@
 #define  COM22_DENOISE		0x10	/* White pixel correction option */
 #define REG_COM23		0x8d	/* Color bar test, color gain */
 #define  COM23_TEST_MODE	0x10
+#define OV9655_REG_COM20	0x8d
+#define  OV9655_COM20_TEST_MODE	0x10
 #define REG_DBLC1		0x8f	/* Digital BLC */
 #define REG_DBLC_B		0x90	/* Digital BLC B channel offset */
 #define REG_DBLC_R		0x91	/* Digital BLC R channel offset */
@@ -193,6 +273,17 @@
 #define REG_AECHM		0xa1	/* Exposure value - bits AEC[15:10] */
 #define REG_BD50ST		0xa2	/* Banding filter value for 50Hz */
 #define REG_BD60ST		0xa3	/* Banding filter value for 60Hz */
+#define OV9655_REG_COM21	0xa4	/* Digital gain */
+#define OV9655_REG_AWB_GREEN	0xa6	/* AWB green */
+#define OV9655_REG_REF_A8	0xa8	/* Analog Reference Control */
+#define OV9655_REG_REF_A9	0xa9	/* Analog Reference Control */
+#define OV9655_REG_BLC(__n)	(0xac + (__n) - 1) /* Black Level Control */
+#define OV9655_REG_CTRLB4	0xb4	/* UV adjustment */
+#define OV9655_REG_ADBOFF	0xbc	/* ADC B channel offset setting */
+#define OV9655_REG_ADROFF	0xbd	/* ADC R channel offset setting */
+#define OV9655_REG_ADGBOFF	0xbe	/* ADC Gb channel offset setting */
+#define OV9655_REG_ADGEOFF	0xbf	/* ADC Gr channel offset setting */
+#define OV9655_REG_COM24	0xc7	/* Pixel clock frequency selection */
 #define REG_NULL		0xff	/* Array end token */
 
 #define DEF_CLKRC		0x80
@@ -200,6 +291,7 @@
 #define OV965X_ID(_msb, _lsb)	((_msb) << 8 | (_lsb))
 #define OV9650_ID		0x9650
 #define OV9652_ID		0x9652
+#define OV9655V5_ID		0x9657
 
 struct ov965x_ctrls {
 	struct v4l2_ctrl_handler handler;
@@ -458,6 +550,292 @@ struct ov965x_pixfmt {
 	{{ 1,   25  }, { QVGA_WIDTH, QVGA_HEIGHT }, 1 },  /* 25 fps */
 };
 
+/* OV9655 */
+static const struct i2c_rv ov9655_init_regs[] = {
+	{ REG_GAIN, 0x00 },
+	{ REG_BLUE, 0x80 },
+	{ REG_RED, 0x80 },
+	{ REG_VREF, 0x02 },
+	{ REG_COM1, 0x03 },
+	{ REG_COM2, 0x01 },/* Output drive x2 */
+	{ REG_COM3, COM3_RGB565 },/* Output drive x2, RGB565 */
+	{ REG_COM5, 0x60 | OV9655_EXPOSURESTEP },/* 0x60 ? */
+	{ REG_COM6, COM6_BLC_OPTICAL },
+	{ REG_CLKRC, 0x01 },/* F(internal clk) = F(input clk) / 2 */
+	{ REG_COM7, OV9655_COM7_VGA | OV9655_COM7_YUV },
+	{ REG_COM8, COM8_FASTAEC | COM8_AECSTEP |
+			COM8_AGC | COM8_AWB | COM8_AEC },
+	{ REG_COM9, COM9_GAIN_CEIL_16X | OV9655_COM9_EXPTIMING |
+			OV9655_COM9_AECDROP },
+	{ OV9655_REG16, OV9655_REG16_DUMMY_8 | 0x4 },
+	{ REG_HSTART, 0x18 },
+	{ REG_HSTOP, 0x04 },
+	{ REG_VSTART, 0x01 },
+	{ REG_VSTOP, 0x81 },
+	{ REG_MVFP, 0x00 },/* No mirror/flip */
+	{ REG_AEW, 0x3c },
+	{ REG_AEB, 0x36 },
+	{ REG_VPT, 0x72 },
+	{ REG_BBIAS, 0x08 },
+	{ REG_GBBIAS, 0x08 },
+	{ OV9655_PREGAIN, 0x15 },
+	{ REG_EXHCH, 0x00 },
+	{ REG_EXHCL, 0x00 },
+	{ REG_RBIAS, 0x08 },
+	{ REG_HREF, 0x12 },/* QVGA */
+	{ REG_CHLF, 0x00 },
+	{ OV9655_AREF1, 0x3f },
+	{ OV9655_AREF2, 0x00 },
+	{ OV9655_AREF3, 0x3a },
+	{ OV9655_ADC2, 0x72 },
+	{ OV9655_AREF4, 0x57 },
+	{ REG_TSLB, OV9655_PCLKDELAY6NS | TSLB_UYVY },
+	{ REG_COM11, 0x04 },/* 0x04 ? */
+	{ REG_COM13, COM13_GAMMA | 0x10 |
+			COM13_Y_DELAY | COM13_UVSWAP },/* 0x10 ? */
+	{OV9655_REG_COM14, OV9655_COM14_ZOOM }, /* QVGA */
+	{ REG_EDGE, 0xc1 },
+	{ REG_COM15, COM15_R00FF },/* Full range output */
+	{ REG_COM16, 0x41 },/* 0x41 ? */
+	{ OV9655_REG_COM17, OV9655_COM17_EDGE_AUTO |
+			OV9655_COM17_DENOISE_AUTO },
+	{ OV9655_REG_RSVD(1), 0x0a },
+	{ OV9655_REG_RSVD(2), 0xf0 },
+	{ OV9655_REG_RSVD(3), 0x46 },
+	{ OV9655_REG_RSVD(4), 0x62 },
+	{ OV9655_REG_RSVD(5), 0x2a },
+	{ OV9655_REG_RSVD(6), 0x3c },
+	{ OV9655_REG_RSVD(7), 0xfc },
+	{ OV9655_REG_RSVD(8), 0xfc },
+	{ OV9655_REG_RSVD(9), 0x7f },
+	{ OV9655_REG_RSVD(10), 0x7f },
+	{ OV9655_REG_RSVD(11), 0x7f },
+	{ REG_MTX(1), 0x98 },
+	{ REG_MTX(2), 0x98 },
+	{ REG_MTX(3), 0x00 },
+	{ REG_MTX(4), 0x28 },
+	{ REG_MTX(5), 0x70 },
+	{ REG_MTX(6), 0x98 },
+	{ REG_MTXS, 0x1a },
+	{ REG_AWBOP(1), 0x85 },
+	{ REG_AWBOP(2), 0xa9 },
+	{ REG_AWBOP(3), 0x64 },
+	{ REG_AWBOP(4), 0x84 },
+	{ REG_AWBOP(5), 0x53 },
+	{ REG_AWBOP(6), 0x0e },
+	{ REG_BLMT, 0xf0 },
+	{ REG_RLMT, 0xf0 },
+	{ REG_GLMT, 0xf0 },
+	{ REG_LCC(1), 0x00 },
+	{ REG_LCC(2), 0x00 },
+	{ REG_LCC(3), 0x02 },
+	{ REG_LCC(4), 0x20 },
+	{ REG_LCC(5), 0x00 },
+	{ 0x69, 0x0a },/* Reserved... */
+	{ OV9655_REG_DBLV, OV9655_DBLV_PLL_4X | OV9655_DBLV_LDO_BYPASS |
+			OV9655_DBLV_BANDGAP },
+	{ 0x6c, 0x04 },/* Reserved... */
+	{ 0x6d, 0x55 },/* Reserved... */
+	{ 0x6e, 0x00 },/* Reserved... */
+	{ 0x6f, 0x9d },/* Reserved... */
+	{ OV9655_REG_DNSTH, 0x21 },
+	{ 0x71, 0x78 },/* Reserved... */
+	{ OV9655_REG_POIDX, 0x11 },/* QVGA */
+	{ OV9655_REG_PCKDV, 0x01 },/* QVGA */
+	{ OV9655_REG_XINDX, 0x10 },
+	{ OV9655_REG_YINDX, 0x10 },
+	{ 0x76, 0x01 },/* Reserved... */
+	{ 0x77, 0x02 },/* Reserved... */
+	{ 0x7A, 0x12 },/* Reserved... */
+	{ OV9655_REG_GAM(1), 0x08 },
+	{ OV9655_REG_GAM(2), 0x16 },
+	{ OV9655_REG_GAM(3), 0x30 },
+	{ OV9655_REG_GAM(4), 0x5e },
+	{ OV9655_REG_GAM(5), 0x72 },
+	{ OV9655_REG_GAM(6), 0x82 },
+	{ OV9655_REG_GAM(7), 0x8e },
+	{ OV9655_REG_GAM(8), 0x9a },
+	{ OV9655_REG_GAM(9), 0xa4 },
+	{ OV9655_REG_GAM(10), 0xac },
+	{ OV9655_REG_GAM(11), 0xb8 },
+	{ OV9655_REG_GAM(12), 0xc3 },
+	{ OV9655_REG_GAM(13), 0xd6 },
+	{ OV9655_REG_GAM(14), 0xe6 },
+	{ OV9655_REG_GAM(15), 0xf2 },
+	{ 0x8a, 0x24 },/* Reserved... */
+	{ OV9655_REG_COM19, 0x80 },
+	{ 0x90, 0x7d },/* Reserved... */
+	{ 0x91, 0x7b },/* Reserved... */
+	{ REG_LCCFB, 0x02 },
+	{ REG_LCCFR, 0x02 },
+	{ REG_DBLC_GB, 0x7a },
+	{ REG_DBLC_GR, 0x79 },
+	{ REG_AECHM, 0x40 },
+	{ OV9655_REG_COM21, 0x50 },
+	{ 0xa5, 0x68 },/* Reserved... */
+	{ OV9655_REG_AWB_GREEN, 0x4a },
+	{ OV9655_REG_REF_A8, 0xc1 },
+	{ OV9655_REG_REF_A9, 0xef },
+	{ 0xaa, 0x92 },/* Reserved... */
+	{ 0xab, 0x04 },/* Reserved... */
+	{ OV9655_REG_BLC(1), 0x80 },
+	{ OV9655_REG_BLC(2), 0x80 },
+	{ OV9655_REG_BLC(3), 0x80 },
+	{ OV9655_REG_BLC(4), 0x80 },
+	{ OV9655_REG_BLC(7), 0xf2 },
+	{ OV9655_REG_BLC(8), 0x20 },
+	{ OV9655_REG_CTRLB4, 0x20 },
+	{ 0xb5, 0x00 },/* Reserved... */
+	{ 0xb6, 0xaf },/* Reserved... */
+	{ 0xb6, 0xaf },/* Reserved... */
+	{ 0xbb, 0xae },/* Reserved... */
+	{ OV9655_REG_ADBOFF, 0x7f },
+	{ OV9655_REG_ADROFF, 0x7f },
+	{ OV9655_REG_ADGBOFF, 0x7f },
+	{ OV9655_REG_ADGEOFF, 0x7f },
+	{ OV9655_REG_ADGEOFF, 0x7f },
+	{ 0xc0, 0xaa },/* Reserved... */
+	{ 0xc1, 0xc0 },/* Reserved... */
+	{ 0xc2, 0x01 },/* Reserved... */
+	{ 0xc3, 0x4e },/* Reserved... */
+	{ 0xc6, 0x05 },/* Reserved... */
+	{ OV9655_REG_COM24, 0x81 },/* QVGA */
+	{ 0xc9, 0xe0 },/* Reserved... */
+	{ 0xca, 0xe8 },/* Reserved... */
+	{ 0xcb, 0xf0 },/* Reserved... */
+	{ 0xcc, 0xd8 },/* Reserved... */
+	{ 0xcd, 0x93 },/* Reserved... */
+	{ REG_COM7, OV9655_COM7_VGA | OV9655_COM7_RGB },
+	{ REG_COM15, COM15_RGB565 },
+	{ REG_NULL, 0}
+};
+
+static const struct i2c_rv ov9655_qvga_regs[] = {
+	{ REG_HREF, 0x12 },
+	{ OV9655_REG_COM14, OV9655_COM14_ZOOM },
+	{ OV9655_REG_POIDX, 0x11 },
+	{ OV9655_REG_PCKDV, 0x01 },
+	{ OV9655_REG_COM24, 0x81 },
+	{ REG_NULL, 0}
+};
+
+static const struct i2c_rv ov9655_qqvga_regs[] = {
+	{ REG_HREF, 0xa4 },
+	{ REG_COM14, OV9655_COM14_BLACK_PIX | OV9655_COM14_WHITE_PIX |
+			OV9655_COM14_ZOOM },
+	{ OV9655_REG_POIDX, 0x22 },
+	{ OV9655_REG_PCKDV, 0x02 },
+	{ OV9655_REG_COM24, 0x82 },
+	{ REG_NULL, 0}
+};
+
+static const struct i2c_rv ov9655_vga_regs[] = {
+	{ REG_GAIN, 0x11 },
+	{ REG_VREF, 0x12 },
+	{ REG_B_AVE, 0x2e },
+	{ REG_GB_AVE, 0x2e },
+	{ REG_GR_AVE, 0x2e },
+	{ REG_R_AVE, 0x2e },
+	{ REG_COM6, 0x48 },
+	{ REG_AECH, 0x7b },
+	{ REG_CLKRC, 0x03 },
+	{ REG_COM8, COM8_FASTAEC | COM8_AECSTEP | COM8_BFILT |
+			COM8_AGC | COM8_AWB | COM8_AEC },
+	{ REG_HSTART, 0x16 },
+	{ REG_HSTOP, 0x02 },
+	{ REG_VSTART, 0x01 },
+	{ REG_VSTOP, 0x3d },
+	{ REG_MVFP, 0x04 },
+	{ REG_YAVE, 0x2e },
+	{ REG_HREF, 0xff },
+	{ OV9655_AREF1, 0x3d },
+	{ OV9655_AREF3, 0xfa },
+	{ REG_TSLB, 0xcc },
+	{ REG_COM11, 0xcc },
+	{ REG_COM14, 0x0c },
+	{ REG_EDGE, 0x82 },
+	{ REG_COM15, COM15_R00FF | COM15_RGB565 },/* full range */
+	{ REG_COM16, 0x40 },
+	{ OV9655_REG_RSVD(1), 0x14 },
+	{ OV9655_REG_RSVD(2), 0xf0 },
+	{ OV9655_REG_RSVD(3), 0x46 },
+	{ OV9655_REG_RSVD(4), 0x62 },
+	{ OV9655_REG_RSVD(5), 0x2a },
+	{ OV9655_REG_RSVD(6), 0x3c },
+	{ OV9655_REG_RSVD(8), 0xe9 },
+	{ OV9655_REG_RSVD(9), 0xdd },
+	{ OV9655_REG_RSVD(10), 0xdd },
+	{ OV9655_REG_RSVD(11), 0xdd },
+	{ OV9655_REG_RSVD(12), 0xdd },
+	{ REG_LCC(1), 0x00 },
+	{ REG_LCC(2), 0x00 },
+	{ REG_LCC(3), 0x02 },
+	{ REG_LCC(4), 0x20 },
+	{ REG_LCC(5), 0x01 },
+	{ REG_GSP, 0x0c },
+	{ 0x6f, 0x9e },/* Reserved... */
+	{ OV9655_REG_DNSTH, 0x06 },
+	{ OV9655_REG_POIDX, 0x00 },
+	{ OV9655_REG_PCKDV, 0x00 },
+	{ OV9655_REG_XINDX, 0x3a },
+	{ OV9655_REG_YINDX, 0x35 },
+	{ OV9655_REG_SLOP, 0x20 },
+	{ OV9655_REG_GAM(1), 0x1c },
+	{ OV9655_REG_GAM(2), 0x28 },
+	{ OV9655_REG_GAM(3), 0x3c },
+	{ OV9655_REG_GAM(4), 0x5a },
+	{ OV9655_REG_GAM(5), 0x68 },
+	{ OV9655_REG_GAM(6), 0x76 },
+	{ OV9655_REG_GAM(7), 0x80 },
+	{ OV9655_REG_GAM(8), 0x88 },
+	{ OV9655_REG_GAM(9), 0x8f },
+	{ OV9655_REG_GAM(10), 0x96 },
+	{ OV9655_REG_GAM(11), 0xa3 },
+	{ OV9655_REG_GAM(12), 0xaf },
+	{ OV9655_REG_GAM(13), 0xc4 },
+	{ OV9655_REG_GAM(14), 0xd7 },
+	{ OV9655_REG_GAM(15), 0xe8 },
+	{ 0x8a, 0x23 },/* Reserved... */
+	{ OV9655_REG_COM19, 0x8d },
+	{ 0x90, 0x92 },/* Reserved... */
+	{ 0x91, 0x92 },/* Reserved... */
+	{ REG_DBLC_GB, 0x90 },
+	{ REG_DBLC_GR, 0x90 },
+	{ OV9655_REG_AWB_GREEN, 0x40 },
+	{ OV9655_REG_ADBOFF, 0x02 },
+	{ OV9655_REG_ADROFF, 0x01 },
+	{ OV9655_REG_ADGBOFF, 0x02 },
+	{ OV9655_REG_ADGEOFF, 0x01 },
+	{ 0xc1, 0xc8 },/* Reserved... */
+	{ 0xc6, 0x85 },/* Reserved... */
+	{ OV9655_REG_COM24, 0x80 },
+	{ REG_NULL, 0}
+};
+
+static const struct ov965x_framesize ov9655_framesizes[] = {
+	{
+		.width		= VGA_WIDTH,
+		.height		= VGA_HEIGHT,
+		.regs		= ov9655_vga_regs,
+		.max_exp_lines	= 498,
+	}, {
+		.width		= QVGA_WIDTH,
+		.height		= QVGA_HEIGHT,
+		.regs		= ov9655_qvga_regs,
+		.max_exp_lines	= 248,
+	},
+	{
+		.width		= QQVGA_WIDTH,
+		.height		= QQVGA_HEIGHT,
+		.regs		= ov9655_qqvga_regs,
+		.max_exp_lines	= 124,
+	},
+};
+
+static const struct ov965x_pixfmt ov9655_formats[] = {
+	{ MEDIA_BUS_FMT_RGB565_2X8_LE, V4L2_COLORSPACE_SRGB, 0x08},
+};
+
 static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
 {
 	return &container_of(ctrl->handler, struct ov965x, ctrls.handler)->sd;
@@ -894,12 +1272,16 @@ static int ov965x_set_test_pattern(struct ov965x *ov965x, int value)
 {
 	int ret;
 	u8 reg;
+	u8 addr = (ov965x->id == OV9655V5_ID) ?
+			REG_COM3 : REG_COM23;
+	u8 mask = (ov965x->id == OV9655V5_ID) ?
+			COM3_COLORBAR : COM23_TEST_MODE;
 
-	ret = ov965x_read(ov965x->client, REG_COM23, &reg);
+	ret = ov965x_read(ov965x->client, addr, &reg);
 	if (ret < 0)
 		return ret;
-	reg = value ? reg | COM23_TEST_MODE : reg & ~COM23_TEST_MODE;
-	return ov965x_write(ov965x->client, REG_COM23, reg);
+	reg = value ? reg | mask : reg & ~mask;
+	return ov965x_write(ov965x->client, addr, reg);
 }
 
 static int __g_volatile_ctrl(struct ov965x *ov965x, struct v4l2_ctrl *ctrl)
@@ -1102,6 +1484,30 @@ static int ov965x_initialize_controls(struct ov965x *ov965x)
 	return 0;
 }
 
+static int ov9655_initialize_controls(struct ov965x *ov965x)
+{
+	const struct v4l2_ctrl_ops *ops = &ov965x_ctrl_ops;
+	struct ov965x_ctrls *ctrls = &ov965x->ctrls;
+	struct v4l2_ctrl_handler *hdl = &ctrls->handler;
+	int ret;
+
+	ret = v4l2_ctrl_handler_init(hdl, 16);
+	if (ret < 0)
+		return ret;
+
+	v4l2_ctrl_new_std_menu_items(hdl, ops, V4L2_CID_TEST_PATTERN,
+				     ARRAY_SIZE(test_pattern_menu) - 1, 0, 0,
+				     test_pattern_menu);
+	if (hdl->error) {
+		ret = hdl->error;
+		v4l2_ctrl_handler_free(hdl);
+		return ret;
+	}
+
+	ov965x->sd.ctrl_handler = hdl;
+	return 0;
+}
+
 /*
  * V4L2 subdev video and pad level operations
  */
@@ -1518,9 +1924,15 @@ static int ov965x_detect_sensor(struct v4l2_subdev *sd)
 
 	if (!ret) {
 		ov965x->id = OV965X_ID(pid, ver);
-		if (ov965x->id == OV9650_ID || ov965x->id == OV9652_ID) {
+		switch (ov965x->id) {
+		case OV9650_ID:
+		case OV9652_ID:
 			v4l2_info(sd, "Found OV%04X sensor\n", ov965x->id);
-		} else {
+			break;
+		case OV9655V5_ID:
+			v4l2_info(sd, "Found OV%04X sensor\n", ov965x->id - 2);
+			break;
+		default:
 			v4l2_err(sd, "Sensor detection failed (%04X, %d)\n",
 				 ov965x->id, ret);
 			ret = -ENODEV;
@@ -1598,18 +2010,28 @@ static int ov965x_probe(struct i2c_client *client,
 	if (ret < 0)
 		goto err_me;
 
-	ov965x->init_regs = ov965x_init_regs;
-	ov965x->initialize_controls = ov965x_initialize_controls;
-	ov965x->framesizes = ov965x_framesizes;
-	ov965x->nb_of_framesizes = ARRAY_SIZE(ov965x_framesizes);
-	ov965x->formats = ov965x_formats;
-	ov965x->nb_of_formats = ARRAY_SIZE(ov965x_formats);
-	ov965x->intervals = ov965x_intervals;
-	ov965x->nb_of_intervals = ARRAY_SIZE(ov965x_intervals);
-	ov965x->fiv = &ov965x_intervals[0];
-	ov965x->set_frame_interval = __ov965x_set_frame_interval;
-	ov965x->update_exposure_ctrl = ov965x_update_exposure_ctrl;
-	ov965x->set_params = __ov965x_set_params;
+	if (ov965x->id != OV9655V5_ID) {
+		ov965x->init_regs = ov965x_init_regs;
+		ov965x->initialize_controls = ov965x_initialize_controls;
+		ov965x->framesizes = ov965x_framesizes;
+		ov965x->nb_of_framesizes = ARRAY_SIZE(ov965x_framesizes);
+		ov965x->formats = ov965x_formats;
+		ov965x->nb_of_formats = ARRAY_SIZE(ov965x_formats);
+		ov965x->intervals = ov965x_intervals;
+		ov965x->nb_of_intervals = ARRAY_SIZE(ov965x_intervals);
+		ov965x->fiv = &ov965x_intervals[0];
+		ov965x->set_frame_interval = __ov965x_set_frame_interval;
+		ov965x->update_exposure_ctrl = ov965x_update_exposure_ctrl;
+		ov965x->set_params = __ov965x_set_params;
+	} else {
+		ov965x->init_regs = ov9655_init_regs;
+		ov965x->initialize_controls = ov9655_initialize_controls;
+		ov965x->framesizes = ov9655_framesizes;
+		ov965x->nb_of_framesizes = ARRAY_SIZE(ov9655_framesizes);
+		ov965x->formats = ov9655_formats;
+		ov965x->nb_of_formats = ARRAY_SIZE(ov9655_formats);
+		ov965x->set_params = ov965x_set_frame_size;
+	};
 
 	ov965x->frame_size = &ov965x->framesizes[0];
 	ov965x_get_default_format(ov965x, &ov965x->format);
@@ -1652,6 +2074,7 @@ static int ov965x_remove(struct i2c_client *client)
 static const struct i2c_device_id ov965x_id[] = {
 	{ "OV9650", 0x9650 },
 	{ "OV9652", 0x9652 },
+	{ "OV9655", 0x9655 },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(i2c, ov965x_id);
@@ -1659,6 +2082,7 @@ static int ov965x_remove(struct i2c_client *client)
 static const struct of_device_id ov965x_of_match[] = {
 	{ .compatible = "ovti,ov9650", .data = (void *)0x9650 },
 	{ .compatible = "ovti,ov9652", .data = (void *)0x9652 },
+	{ .compatible = "ovti,ov9655", .data = (void *)0x9655 },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, ov965x_of_match);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1674246 — Re: [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant

From	kbuild test robot <lkp@intel.com>
Date	2017-06-25 18:10 +0200
Subject	Re: [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant
Message-ID	<tWigN-7fN-7@gated-at.bofh.it>
In reply to	#1672734

Hi Hugues,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.12-rc6 next-20170623]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Hugues-Fruchet/Add-support-of-OV9655-camera/20170625-201153
base:   git://linuxtv.org/media_tree.git master


coccinelle warnings: (new ones prefixed by >>)

>> drivers/media/i2c/ov9650.c:2034:2-3: Unneeded semicolon

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1674247 — [PATCH] ov9650: fix semicolon.cocci warnings

From	kbuild test robot <lkp@intel.com>
Date	2017-06-25 18:10 +0200
Subject	[PATCH] ov9650: fix semicolon.cocci warnings
Message-ID	<tWigN-7fN-5@gated-at.bofh.it>
In reply to	#1672734

drivers/media/i2c/ov9650.c:2034:2-3: Unneeded semicolon


 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

Fixes: 5ffa34fa8f2e ("ov9650: add support of OV9655 variant")
CC: Hugues Fruchet <hugues.fruchet@st.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 ov9650.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -2031,7 +2031,7 @@ static int ov965x_probe(struct i2c_clien
 		ov965x->formats = ov9655_formats;
 		ov965x->nb_of_formats = ARRAY_SIZE(ov9655_formats);
 		ov965x->set_params = ov965x_set_frame_size;
-	};
+	}
 
 	ov965x->frame_size = &ov965x->framesizes[0];
 	ov965x_get_default_format(ov965x, &ov965x->format);

[toc] | [prev] | [next] | [standalone]

#1674448 — Re: [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant

From	"H. Nikolaus Schaller" <hns@goldelico.com>
Date	2017-06-26 08:10 +0200
Subject	Re: [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant
Message-ID	<tWvnH-76i-15@gated-at.bofh.it>
In reply to	#1672734

> Am 22.06.2017 um 17:05 schrieb Hugues Fruchet <hugues.fruchet@st.com>:
> 
> Add a first support of OV9655 variant.
> Because of register set slightly different from OV9650/9652,
> not all of the driver features are supported (controls).
> Supported resolutions are limited to VGA, QVGA, QQVGA.
> Supported format is limited to RGB565.
> Controls are limited to color bar test pattern for test purpose.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
> ---
> drivers/media/i2c/Kconfig  |   4 +-
> drivers/media/i2c/ov9650.c | 486 ++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 457 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index efea14d..a8f638c 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -594,11 +594,11 @@ config VIDEO_OV7670
> 	  controller.
> 
> config VIDEO_OV9650
> -	tristate "OmniVision OV9650/OV9652 sensor support"
> +	tristate "OmniVision OV9650/OV9652/OV9655 sensor support"
> 	depends on GPIOLIB && I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> 	---help---
> 	  This is a V4L2 sensor-level driver for the Omnivision
> -	  OV9650 and OV9652 camera sensors.
> +	  OV9650 and OV9652 and OV9655 camera sensors.
> 
> config VIDEO_VS6624
> 	tristate "ST VS6624 sensor support"
> diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
> index a9d268d..c0819af 100644
> --- a/drivers/media/i2c/ov9650.c
> +++ b/drivers/media/i2c/ov9650.c
> @@ -1,5 +1,5 @@
> /*
> - * Omnivision OV9650/OV9652 CMOS Image Sensor driver
> + * Omnivision OV9650/OV9652/OV9655 CMOS Image Sensor driver
>  *
>  * Copyright (C) 2013, Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
>  *
> @@ -7,6 +7,15 @@
>  * by Vladimir Fonov.
>  * Copyright (c) 2010, Vladimir Fonov
>  *
> + *
> + * Copyright (C) STMicroelectronics SA 2017
> + * Author: Hugues Fruchet <hugues.fruchet@st.com> for STMicroelectronics.
> + *
> + * OV9655 initial support based on a driver written by H. Nikolaus Schaller:
> + *   http://git.goldelico.com/?p=gta04-kernel.git;a=shortlog;h=refs/heads/work/hns/video/ov9655
> + * OV9655 registers sequence from STM32CubeF7 embedded software, see:
> + *   https://developer.mbed.org/teams/ST/code/BSP_DISCO_F746NG/file/e1d9da7fe856/Drivers/BSP/Components/ov9655/ov9655.c
> + *
>  * This program is free software; you can redistribute it and/or modify
>  * it under the terms of the GNU General Public License version 2 as
>  * published by the Free Software Foundation.
> @@ -58,14 +67,21 @@
> #define REG_PID			0x0a	/* Product ID MSB */
> #define REG_VER			0x0b	/* Product ID LSB */
> #define REG_COM3		0x0c
> -#define  COM3_SWAP		0x40
> +#define  COM3_COLORBAR		0x80
> +#define  COM3_RGB565		0x00
> +#define  COM3_SWAP		0x40	/* Doesn't work in RGB */
> +#define  COM3_RESETB		0x08
> #define  COM3_VARIOPIXEL1	0x04
> +#define  OV9655_SINGLEFRAME	0x01
> #define REG_COM4		0x0d	/* Vario Pixels  */
> #define  COM4_VARIOPIXEL2	0x80
> +#define  OV9655_TRISTATE		/* seems to have a different function */
> #define REG_COM5		0x0e	/* System clock options */
> #define  COM5_SLAVE_MODE	0x10
> -#define  COM5_SYSTEMCLOCK48MHZ	0x80
> +#define  COM5_SYSTEMCLOCK48MHZ	0x80	/* not on OV9655 */
> +#define  OV9655_EXPOSURESTEP	0x01
> #define REG_COM6		0x0f	/* HREF & ADBLC options */
> +#define  COM6_BLC_OPTICAL	0x40	/* Optical black */
> #define REG_AECH		0x10	/* Exposure value, AEC[9:2] */
> #define REG_CLKRC		0x11	/* Clock control */
> #define  CLK_EXT		0x40	/* Use external clock directly */
> @@ -74,13 +90,18 @@
> #define  COM7_RESET		0x80
> #define  COM7_FMT_MASK		0x38
> #define  COM7_FMT_VGA		0x40
> -#define	 COM7_FMT_CIF		0x20
> +#define  COM7_FMT_CIF		0x20
> #define  COM7_FMT_QVGA		0x10
> #define  COM7_FMT_QCIF		0x08
> -#define	 COM7_RGB		0x04
> -#define	 COM7_YUV		0x00
> -#define	 COM7_BAYER		0x01
> -#define	 COM7_PBAYER		0x05
> +#define  COM7_RGB		0x04
> +#define  COM7_YUV		0x00
> +#define  COM7_BAYER		0x01
> +#define  COM7_PBAYER		0x05
> +#define  OV9655_COM7_VGA	0x60
> +#define  OV9655_COM7_RAWRGB	0x00	/* different format encoding */
> +#define  OV9655_COM7_RAWRGBINT	0x01
> +#define  OV9655_COM7_YUV	0x02
> +#define  OV9655_COM7_RGB	0x03
> #define REG_COM8		0x13	/* AGC/AEC options */
> #define  COM8_FASTAEC		0x80	/* Enable fast AGC/AEC */
> #define  COM8_AECSTEP		0x40	/* Unlimited AEC step size */
> @@ -89,14 +110,23 @@
> #define  COM8_AWB		0x02	/* White balance enable */
> #define  COM8_AEC		0x01	/* Auto exposure enable */
> #define REG_COM9		0x14	/* Gain ceiling */
> -#define  COM9_GAIN_CEIL_MASK	0x70	/* */
> +#define  COM9_GAIN_CEIL_MASK	0x70
> +#define  COM9_GAIN_CEIL_16X	0x30
> +#define  OV9655_COM9_EXPTIMING	0x08
> +#define  OV9655_COM9_VSYNCDROP	0x04
> +#define  OV9655_COM9_AECDROP	0x02
> #define REG_COM10		0x15	/* PCLK, HREF, HSYNC signals polarity */
> +#define  OV9655_SLAVE_PIN	0x80	/* SLHS/SLVS instead of RESETB/PWDN */
> #define  COM10_HSYNC		0x40	/* HSYNC instead of HREF */
> #define  COM10_PCLK_HB		0x20	/* Suppress PCLK on horiz blank */
> -#define  COM10_HREF_REV		0x08	/* Reverse HREF */
> +#define  OV9655_COM10_PCLK_REV		0x10	/* PCLK reverse */
> +#define  COM10_HREF_REV	0x08	/* Reverse HREF */
> #define  COM10_VS_LEAD		0x04	/* VSYNC on clock leading edge */
> +#define  OV9655_COM10_RESET_OPTION	0x04	/* Reset signal end point */
> #define  COM10_VS_NEG		0x02	/* VSYNC negative */
> #define  COM10_HS_NEG		0x01	/* HSYNC negative */
> +#define OV9655_REG16		0x16	/* dummy frame and blanking */
> +#define   OV9655_REG16_DUMMY_8	0x20	/* dummy frame when gain > 8 */
> #define REG_HSTART		0x17	/* Horiz start high bits */
> #define REG_HSTOP		0x18	/* Horiz stop high bits */
> #define REG_VSTART		0x19	/* Vert start high bits */
> @@ -117,6 +147,7 @@
> #define REG_BBIAS		0x27	/* B channel output bias */
> #define REG_GBBIAS		0x28	/* Gb channel output bias */
> #define REG_GRCOM		0x29	/* Analog BLC & regulator */
> +#define OV9655_PREGAIN		0x29
> #define REG_EXHCH		0x2a	/* Dummy pixel insert MSB */
> #define REG_EXHCL		0x2b	/* Dummy pixel insert LSB */
> #define REG_RBIAS		0x2c	/* R channel output bias */
> @@ -127,12 +158,30 @@
> #define REG_HSYEN		0x31	/* HSYNC falling edge delay LSB*/
> #define REG_HREF		0x32	/* HREF pieces */
> #define REG_CHLF		0x33	/* reserved */
> +#define OV9655_CLKF		0x33	/* Array current control */
> +#define OV9655_AREF1		0x34	/* Array reference control */
> +#define OV9655_AREF2		0x35	/* Array reference control */
> +#define OV9655_AREF3		0x36	/* Array reference control */
> #define REG_ADC			0x37	/* reserved */
> +#define OV9655_ADC		0x37	/* ADC Control 1 (Range adjustment) */
> #define REG_ACOM		0x38	/* reserved */
> -#define REG_OFON		0x39	/* Power down register */
> +#define OV9655_ADC2		0x38	/* ADC Control 2 (Range adjustment) */
> +#define REG_OFON		0x39	/* Power down register (ov9650 only) */
> #define  OFON_PWRDN		0x08	/* Power down bit */
> +#define OV9655_AREF4		0x39	/* Array reference control */
> #define REG_TSLB		0x3a	/* YUVU format */
> +#define  OV9655_PCLKDELAY2NS	0x40
> +#define  OV9655_PCLKDELAY4NS	0x80
> +#define  OV9655_PCLKDELAY6NS	0xc0
> +#define  OV9655_OUTREVERSE	0x20
> +#define  OV9655_FIXEDUV	0x10
> #define  TSLB_YUYV_MASK		0x0c	/* UYVY or VYUY - see com13 */
> +#define  TSLB_YUYV		0x00
> +#define  TSLB_YVYU		0x04
> +#define  TSLB_VYUY		0x08
> +#define  TSLB_UYVY		0x0c
> +#define  OV9655_BANDINGAUTO	0x02
> +
> #define REG_COM11		0x3b	/* Night mode, banding filter enable */
> #define  COM11_NIGHT		0x80	/* Night mode enable */
> #define  COM11_NMFR		0x60	/* Two bit NM frame rate */
> @@ -142,25 +191,38 @@
> #define  COM12_HREF		0x80	/* HREF always */
> #define REG_COM13		0x3d	/* Gamma selection, Color matrix en. */
> #define  COM13_GAMMA		0x80	/* Gamma enable */
> -#define	 COM13_UVSAT		0x40	/* UV saturation auto adjustment */
> +#define  COM13_UVSAT		0x40	/* UV saturation auto adjustment */
> +#define  COM13_Y_DELAY		0x08	/* Delay Y channel */
> #define  COM13_UVSWAP		0x01	/* V before U - w/TSLB */
> #define REG_COM14		0x3e	/* Edge enhancement options */
> #define  COM14_EDGE_EN		0x02
> #define  COM14_EEF_X2		0x01
> +#define OV9655_REG_COM14	0x3e	/* pixel correction/zoom ON/OFF sel. */
> +#define  OV9655_COM14_BLACK_PIX	0x08	/* Black pixel correction */
> +#define  OV9655_COM14_WHITE_PIX	0x04	/* White pixel correction */
> +#define  OV9655_COM14_ZOOM	0x02	/* Zoom function ON */
> #define REG_EDGE		0x3f	/* Edge enhancement factor */
> #define  EDGE_FACTOR_MASK	0x0f
> #define REG_COM15		0x40	/* Output range, RGB 555/565 */
> #define  COM15_R10F0		0x00	/* Data range 10 to F0 */
> -#define	 COM15_R01FE		0x80	/* 01 to FE */
> +#define  COM15_R01FE		0x80	/* 01 to FE */
> #define  COM15_R00FF		0xc0	/* 00 to FF */
> #define  COM15_RGB565		0x10	/* RGB565 output */
> #define  COM15_RGB555		0x30	/* RGB555 output */
> #define  COM15_SWAPRB		0x04	/* Swap R&B */
> #define REG_COM16		0x41	/* Color matrix coeff options */
> #define REG_COM17		0x42	/* Single frame out, banding filter */
> +#define OV9655_REG_COM17	0x42	/* Denoise, edge, auto gain, ... */
> +#define   OV9655_COM17_EDGE_AUTO	0x40	/* Edge auto */
> +#define   OV9655_COM17_DENOISE_AUTO	0x80	/* Denoise auto */
> +#define OV9655_REG_RSVD(__n)	(0x43 + (__n) - 1) /* reserved but used... */
> /* n = 1...9, 0x4f..0x57 */
> -#define	REG_MTX(__n)		(0x4f + (__n) - 1)
> +#define REG_MTX(__n)		(0x4f + (__n) - 1)
> #define REG_MTXS		0x58
> +#define REG_AWBOP(__n)		(0x59 + (__n) - 1) /* AWB control options */
> +#define REG_BLMT		0x5F	/* AWB Blue Component Gain Limit */
> +#define REG_RLMT		0x60	/* AWB Red Component Gain Limit */
> +#define REG_GLMT		0x61	/* AWB Green Component Gain Limit */
> /* Lens Correction Option 1...5, __n = 0...5 */
> #define REG_LCC(__n)		(0x62 + (__n) - 1)
> #define  LCC5_LCC_ENABLE	0x01	/* LCC5, enable lens correction */
> @@ -170,10 +232,26 @@
> #define REG_HV			0x69	/* Manual banding filter MSB */
> #define REG_MBD			0x6a	/* Manual banding filter value */
> #define REG_DBLV		0x6b	/* reserved */
> +#define OV9655_REG_DBLV		0x6b	/* PLL, DVDD regu bypass, bandgap */
> +#define  OV9655_DBLV_BANDGAP	0x0a	/* default value */
> +#define  OV9655_DBLV_LDO_BYPASS	0x10
> +#define  OV9655_DBLV_PLL_BYPASS	0x00
> +#define  OV9655_DBLV_PLL_4X	0x40
> +#define  OV9655_DBLV_PLL_6X	0x80
> +#define  OV9655_DBLV_PLL_8X	0xc0
> #define REG_GSP			0x6c	/* Gamma curve */
> #define  GSP_LEN		15
> +#define OV9655_REG_DNSTH	0x70	/* De-noise Function Threshold Adj. */
> +#define OV9655_REG_POIDX	0x72	/* Pixel output index */
> +#define OV9655_REG_PCKDV	0x73	/* Pixel Clock Output Selection */
> +#define OV9655_REG_XINDX	0x74	/* Horizontal Scaling Down Coeff. */
> +#define OV9655_REG_YINDX	0x75	/* Vertical Scaling Down Coeff. */
> +#define OV9655_REG_SLOP		0x7A	/* Gamma Curve Highest Segment Slope */
> +#define OV9655_REG_GAM(__n)	(0x7B + (__n) - 1)	/* Gamma curve */
> #define REG_GST			0x7c	/* Gamma curve */
> #define  GST_LEN		15
> +#define OV9655_REG_COM18	0x8b	/* Zoom mode in VGA */
> +#define OV9655_REG_COM19	0x8c	/* UV adjustment */
> #define REG_COM21		0x8b
> #define REG_COM22		0x8c	/* Edge enhancement, denoising */
> #define  COM22_WHTPCOR		0x02	/* White pixel correction enable */
> @@ -181,6 +259,8 @@
> #define  COM22_DENOISE		0x10	/* White pixel correction option */
> #define REG_COM23		0x8d	/* Color bar test, color gain */
> #define  COM23_TEST_MODE	0x10
> +#define OV9655_REG_COM20	0x8d
> +#define  OV9655_COM20_TEST_MODE	0x10
> #define REG_DBLC1		0x8f	/* Digital BLC */
> #define REG_DBLC_B		0x90	/* Digital BLC B channel offset */
> #define REG_DBLC_R		0x91	/* Digital BLC R channel offset */
> @@ -193,6 +273,17 @@
> #define REG_AECHM		0xa1	/* Exposure value - bits AEC[15:10] */
> #define REG_BD50ST		0xa2	/* Banding filter value for 50Hz */
> #define REG_BD60ST		0xa3	/* Banding filter value for 60Hz */
> +#define OV9655_REG_COM21	0xa4	/* Digital gain */
> +#define OV9655_REG_AWB_GREEN	0xa6	/* AWB green */
> +#define OV9655_REG_REF_A8	0xa8	/* Analog Reference Control */
> +#define OV9655_REG_REF_A9	0xa9	/* Analog Reference Control */
> +#define OV9655_REG_BLC(__n)	(0xac + (__n) - 1) /* Black Level Control */
> +#define OV9655_REG_CTRLB4	0xb4	/* UV adjustment */
> +#define OV9655_REG_ADBOFF	0xbc	/* ADC B channel offset setting */
> +#define OV9655_REG_ADROFF	0xbd	/* ADC R channel offset setting */
> +#define OV9655_REG_ADGBOFF	0xbe	/* ADC Gb channel offset setting */
> +#define OV9655_REG_ADGEOFF	0xbf	/* ADC Gr channel offset setting */
> +#define OV9655_REG_COM24	0xc7	/* Pixel clock frequency selection */
> #define REG_NULL		0xff	/* Array end token */
> 
> #define DEF_CLKRC		0x80
> @@ -200,6 +291,7 @@
> #define OV965X_ID(_msb, _lsb)	((_msb) << 8 | (_lsb))
> #define OV9650_ID		0x9650
> #define OV9652_ID		0x9652
> +#define OV9655V5_ID		0x9657
> 
> struct ov965x_ctrls {
> 	struct v4l2_ctrl_handler handler;
> @@ -458,6 +550,292 @@ struct ov965x_pixfmt {
> 	{{ 1,   25  }, { QVGA_WIDTH, QVGA_HEIGHT }, 1 },  /* 25 fps */
> };
> 
> +/* OV9655 */
> +static const struct i2c_rv ov9655_init_regs[] = {
> +	{ REG_GAIN, 0x00 },
> +	{ REG_BLUE, 0x80 },
> +	{ REG_RED, 0x80 },
> +	{ REG_VREF, 0x02 },
> +	{ REG_COM1, 0x03 },
> +	{ REG_COM2, 0x01 },/* Output drive x2 */
> +	{ REG_COM3, COM3_RGB565 },/* Output drive x2, RGB565 */
> +	{ REG_COM5, 0x60 | OV9655_EXPOSURESTEP },/* 0x60 ? */
> +	{ REG_COM6, COM6_BLC_OPTICAL },
> +	{ REG_CLKRC, 0x01 },/* F(internal clk) = F(input clk) / 2 */
> +	{ REG_COM7, OV9655_COM7_VGA | OV9655_COM7_YUV },
> +	{ REG_COM8, COM8_FASTAEC | COM8_AECSTEP |
> +			COM8_AGC | COM8_AWB | COM8_AEC },
> +	{ REG_COM9, COM9_GAIN_CEIL_16X | OV9655_COM9_EXPTIMING |
> +			OV9655_COM9_AECDROP },
> +	{ OV9655_REG16, OV9655_REG16_DUMMY_8 | 0x4 },
> +	{ REG_HSTART, 0x18 },
> +	{ REG_HSTOP, 0x04 },
> +	{ REG_VSTART, 0x01 },
> +	{ REG_VSTOP, 0x81 },
> +	{ REG_MVFP, 0x00 },/* No mirror/flip */
> +	{ REG_AEW, 0x3c },
> +	{ REG_AEB, 0x36 },
> +	{ REG_VPT, 0x72 },
> +	{ REG_BBIAS, 0x08 },
> +	{ REG_GBBIAS, 0x08 },
> +	{ OV9655_PREGAIN, 0x15 },
> +	{ REG_EXHCH, 0x00 },
> +	{ REG_EXHCL, 0x00 },
> +	{ REG_RBIAS, 0x08 },
> +	{ REG_HREF, 0x12 },/* QVGA */
> +	{ REG_CHLF, 0x00 },
> +	{ OV9655_AREF1, 0x3f },
> +	{ OV9655_AREF2, 0x00 },
> +	{ OV9655_AREF3, 0x3a },
> +	{ OV9655_ADC2, 0x72 },
> +	{ OV9655_AREF4, 0x57 },
> +	{ REG_TSLB, OV9655_PCLKDELAY6NS | TSLB_UYVY },
> +	{ REG_COM11, 0x04 },/* 0x04 ? */
> +	{ REG_COM13, COM13_GAMMA | 0x10 |
> +			COM13_Y_DELAY | COM13_UVSWAP },/* 0x10 ? */
> +	{OV9655_REG_COM14, OV9655_COM14_ZOOM }, /* QVGA */
> +	{ REG_EDGE, 0xc1 },
> +	{ REG_COM15, COM15_R00FF },/* Full range output */
> +	{ REG_COM16, 0x41 },/* 0x41 ? */
> +	{ OV9655_REG_COM17, OV9655_COM17_EDGE_AUTO |
> +			OV9655_COM17_DENOISE_AUTO },
> +	{ OV9655_REG_RSVD(1), 0x0a },
> +	{ OV9655_REG_RSVD(2), 0xf0 },
> +	{ OV9655_REG_RSVD(3), 0x46 },
> +	{ OV9655_REG_RSVD(4), 0x62 },
> +	{ OV9655_REG_RSVD(5), 0x2a },
> +	{ OV9655_REG_RSVD(6), 0x3c },
> +	{ OV9655_REG_RSVD(7), 0xfc },
> +	{ OV9655_REG_RSVD(8), 0xfc },
> +	{ OV9655_REG_RSVD(9), 0x7f },
> +	{ OV9655_REG_RSVD(10), 0x7f },
> +	{ OV9655_REG_RSVD(11), 0x7f },
> +	{ REG_MTX(1), 0x98 },
> +	{ REG_MTX(2), 0x98 },
> +	{ REG_MTX(3), 0x00 },
> +	{ REG_MTX(4), 0x28 },
> +	{ REG_MTX(5), 0x70 },
> +	{ REG_MTX(6), 0x98 },
> +	{ REG_MTXS, 0x1a },
> +	{ REG_AWBOP(1), 0x85 },
> +	{ REG_AWBOP(2), 0xa9 },
> +	{ REG_AWBOP(3), 0x64 },
> +	{ REG_AWBOP(4), 0x84 },
> +	{ REG_AWBOP(5), 0x53 },
> +	{ REG_AWBOP(6), 0x0e },
> +	{ REG_BLMT, 0xf0 },
> +	{ REG_RLMT, 0xf0 },
> +	{ REG_GLMT, 0xf0 },
> +	{ REG_LCC(1), 0x00 },
> +	{ REG_LCC(2), 0x00 },
> +	{ REG_LCC(3), 0x02 },
> +	{ REG_LCC(4), 0x20 },
> +	{ REG_LCC(5), 0x00 },
> +	{ 0x69, 0x0a },/* Reserved... */
> +	{ OV9655_REG_DBLV, OV9655_DBLV_PLL_4X | OV9655_DBLV_LDO_BYPASS |
> +			OV9655_DBLV_BANDGAP },
> +	{ 0x6c, 0x04 },/* Reserved... */
> +	{ 0x6d, 0x55 },/* Reserved... */
> +	{ 0x6e, 0x00 },/* Reserved... */
> +	{ 0x6f, 0x9d },/* Reserved... */
> +	{ OV9655_REG_DNSTH, 0x21 },
> +	{ 0x71, 0x78 },/* Reserved... */
> +	{ OV9655_REG_POIDX, 0x11 },/* QVGA */
> +	{ OV9655_REG_PCKDV, 0x01 },/* QVGA */
> +	{ OV9655_REG_XINDX, 0x10 },
> +	{ OV9655_REG_YINDX, 0x10 },
> +	{ 0x76, 0x01 },/* Reserved... */
> +	{ 0x77, 0x02 },/* Reserved... */
> +	{ 0x7A, 0x12 },/* Reserved... */
> +	{ OV9655_REG_GAM(1), 0x08 },
> +	{ OV9655_REG_GAM(2), 0x16 },
> +	{ OV9655_REG_GAM(3), 0x30 },
> +	{ OV9655_REG_GAM(4), 0x5e },
> +	{ OV9655_REG_GAM(5), 0x72 },
> +	{ OV9655_REG_GAM(6), 0x82 },
> +	{ OV9655_REG_GAM(7), 0x8e },
> +	{ OV9655_REG_GAM(8), 0x9a },
> +	{ OV9655_REG_GAM(9), 0xa4 },
> +	{ OV9655_REG_GAM(10), 0xac },
> +	{ OV9655_REG_GAM(11), 0xb8 },
> +	{ OV9655_REG_GAM(12), 0xc3 },
> +	{ OV9655_REG_GAM(13), 0xd6 },
> +	{ OV9655_REG_GAM(14), 0xe6 },
> +	{ OV9655_REG_GAM(15), 0xf2 },
> +	{ 0x8a, 0x24 },/* Reserved... */
> +	{ OV9655_REG_COM19, 0x80 },
> +	{ 0x90, 0x7d },/* Reserved... */
> +	{ 0x91, 0x7b },/* Reserved... */
> +	{ REG_LCCFB, 0x02 },
> +	{ REG_LCCFR, 0x02 },
> +	{ REG_DBLC_GB, 0x7a },
> +	{ REG_DBLC_GR, 0x79 },
> +	{ REG_AECHM, 0x40 },
> +	{ OV9655_REG_COM21, 0x50 },
> +	{ 0xa5, 0x68 },/* Reserved... */
> +	{ OV9655_REG_AWB_GREEN, 0x4a },
> +	{ OV9655_REG_REF_A8, 0xc1 },
> +	{ OV9655_REG_REF_A9, 0xef },
> +	{ 0xaa, 0x92 },/* Reserved... */
> +	{ 0xab, 0x04 },/* Reserved... */
> +	{ OV9655_REG_BLC(1), 0x80 },
> +	{ OV9655_REG_BLC(2), 0x80 },
> +	{ OV9655_REG_BLC(3), 0x80 },
> +	{ OV9655_REG_BLC(4), 0x80 },
> +	{ OV9655_REG_BLC(7), 0xf2 },
> +	{ OV9655_REG_BLC(8), 0x20 },
> +	{ OV9655_REG_CTRLB4, 0x20 },
> +	{ 0xb5, 0x00 },/* Reserved... */
> +	{ 0xb6, 0xaf },/* Reserved... */
> +	{ 0xb6, 0xaf },/* Reserved... */
> +	{ 0xbb, 0xae },/* Reserved... */
> +	{ OV9655_REG_ADBOFF, 0x7f },
> +	{ OV9655_REG_ADROFF, 0x7f },
> +	{ OV9655_REG_ADGBOFF, 0x7f },
> +	{ OV9655_REG_ADGEOFF, 0x7f },
> +	{ OV9655_REG_ADGEOFF, 0x7f },
> +	{ 0xc0, 0xaa },/* Reserved... */
> +	{ 0xc1, 0xc0 },/* Reserved... */
> +	{ 0xc2, 0x01 },/* Reserved... */
> +	{ 0xc3, 0x4e },/* Reserved... */
> +	{ 0xc6, 0x05 },/* Reserved... */
> +	{ OV9655_REG_COM24, 0x81 },/* QVGA */
> +	{ 0xc9, 0xe0 },/* Reserved... */
> +	{ 0xca, 0xe8 },/* Reserved... */
> +	{ 0xcb, 0xf0 },/* Reserved... */
> +	{ 0xcc, 0xd8 },/* Reserved... */
> +	{ 0xcd, 0x93 },/* Reserved... */
> +	{ REG_COM7, OV9655_COM7_VGA | OV9655_COM7_RGB },
> +	{ REG_COM15, COM15_RGB565 },
> +	{ REG_NULL, 0}
> +};
> +
> +static const struct i2c_rv ov9655_qvga_regs[] = {
> +	{ REG_HREF, 0x12 },
> +	{ OV9655_REG_COM14, OV9655_COM14_ZOOM },
> +	{ OV9655_REG_POIDX, 0x11 },
> +	{ OV9655_REG_PCKDV, 0x01 },
> +	{ OV9655_REG_COM24, 0x81 },
> +	{ REG_NULL, 0}
> +};
> +
> +static const struct i2c_rv ov9655_qqvga_regs[] = {
> +	{ REG_HREF, 0xa4 },
> +	{ REG_COM14, OV9655_COM14_BLACK_PIX | OV9655_COM14_WHITE_PIX |
> +			OV9655_COM14_ZOOM },
> +	{ OV9655_REG_POIDX, 0x22 },
> +	{ OV9655_REG_PCKDV, 0x02 },
> +	{ OV9655_REG_COM24, 0x82 },
> +	{ REG_NULL, 0}
> +};
> +
> +static const struct i2c_rv ov9655_vga_regs[] = {
> +	{ REG_GAIN, 0x11 },
> +	{ REG_VREF, 0x12 },
> +	{ REG_B_AVE, 0x2e },
> +	{ REG_GB_AVE, 0x2e },
> +	{ REG_GR_AVE, 0x2e },
> +	{ REG_R_AVE, 0x2e },
> +	{ REG_COM6, 0x48 },
> +	{ REG_AECH, 0x7b },
> +	{ REG_CLKRC, 0x03 },
> +	{ REG_COM8, COM8_FASTAEC | COM8_AECSTEP | COM8_BFILT |
> +			COM8_AGC | COM8_AWB | COM8_AEC },
> +	{ REG_HSTART, 0x16 },
> +	{ REG_HSTOP, 0x02 },
> +	{ REG_VSTART, 0x01 },
> +	{ REG_VSTOP, 0x3d },
> +	{ REG_MVFP, 0x04 },
> +	{ REG_YAVE, 0x2e },
> +	{ REG_HREF, 0xff },
> +	{ OV9655_AREF1, 0x3d },
> +	{ OV9655_AREF3, 0xfa },
> +	{ REG_TSLB, 0xcc },
> +	{ REG_COM11, 0xcc },
> +	{ REG_COM14, 0x0c },
> +	{ REG_EDGE, 0x82 },
> +	{ REG_COM15, COM15_R00FF | COM15_RGB565 },/* full range */
> +	{ REG_COM16, 0x40 },
> +	{ OV9655_REG_RSVD(1), 0x14 },
> +	{ OV9655_REG_RSVD(2), 0xf0 },
> +	{ OV9655_REG_RSVD(3), 0x46 },
> +	{ OV9655_REG_RSVD(4), 0x62 },
> +	{ OV9655_REG_RSVD(5), 0x2a },
> +	{ OV9655_REG_RSVD(6), 0x3c },
> +	{ OV9655_REG_RSVD(8), 0xe9 },
> +	{ OV9655_REG_RSVD(9), 0xdd },
> +	{ OV9655_REG_RSVD(10), 0xdd },
> +	{ OV9655_REG_RSVD(11), 0xdd },
> +	{ OV9655_REG_RSVD(12), 0xdd },
> +	{ REG_LCC(1), 0x00 },
> +	{ REG_LCC(2), 0x00 },
> +	{ REG_LCC(3), 0x02 },
> +	{ REG_LCC(4), 0x20 },
> +	{ REG_LCC(5), 0x01 },
> +	{ REG_GSP, 0x0c },
> +	{ 0x6f, 0x9e },/* Reserved... */
> +	{ OV9655_REG_DNSTH, 0x06 },
> +	{ OV9655_REG_POIDX, 0x00 },
> +	{ OV9655_REG_PCKDV, 0x00 },
> +	{ OV9655_REG_XINDX, 0x3a },
> +	{ OV9655_REG_YINDX, 0x35 },
> +	{ OV9655_REG_SLOP, 0x20 },
> +	{ OV9655_REG_GAM(1), 0x1c },
> +	{ OV9655_REG_GAM(2), 0x28 },
> +	{ OV9655_REG_GAM(3), 0x3c },
> +	{ OV9655_REG_GAM(4), 0x5a },
> +	{ OV9655_REG_GAM(5), 0x68 },
> +	{ OV9655_REG_GAM(6), 0x76 },
> +	{ OV9655_REG_GAM(7), 0x80 },
> +	{ OV9655_REG_GAM(8), 0x88 },
> +	{ OV9655_REG_GAM(9), 0x8f },
> +	{ OV9655_REG_GAM(10), 0x96 },
> +	{ OV9655_REG_GAM(11), 0xa3 },
> +	{ OV9655_REG_GAM(12), 0xaf },
> +	{ OV9655_REG_GAM(13), 0xc4 },
> +	{ OV9655_REG_GAM(14), 0xd7 },
> +	{ OV9655_REG_GAM(15), 0xe8 },
> +	{ 0x8a, 0x23 },/* Reserved... */
> +	{ OV9655_REG_COM19, 0x8d },
> +	{ 0x90, 0x92 },/* Reserved... */
> +	{ 0x91, 0x92 },/* Reserved... */
> +	{ REG_DBLC_GB, 0x90 },
> +	{ REG_DBLC_GR, 0x90 },
> +	{ OV9655_REG_AWB_GREEN, 0x40 },
> +	{ OV9655_REG_ADBOFF, 0x02 },
> +	{ OV9655_REG_ADROFF, 0x01 },
> +	{ OV9655_REG_ADGBOFF, 0x02 },
> +	{ OV9655_REG_ADGEOFF, 0x01 },
> +	{ 0xc1, 0xc8 },/* Reserved... */
> +	{ 0xc6, 0x85 },/* Reserved... */
> +	{ OV9655_REG_COM24, 0x80 },
> +	{ REG_NULL, 0}
> +};
> +
> +static const struct ov965x_framesize ov9655_framesizes[] = {
> +	{
> +		.width		= VGA_WIDTH,
> +		.height		= VGA_HEIGHT,
> +		.regs		= ov9655_vga_regs,
> +		.max_exp_lines	= 498,
> +	}, {
> +		.width		= QVGA_WIDTH,
> +		.height		= QVGA_HEIGHT,
> +		.regs		= ov9655_qvga_regs,
> +		.max_exp_lines	= 248,
> +	},
> +	{
> +		.width		= QQVGA_WIDTH,
> +		.height		= QQVGA_HEIGHT,
> +		.regs		= ov9655_qqvga_regs,
> +		.max_exp_lines	= 124,
> +	},
> +};
> +
> +static const struct ov965x_pixfmt ov9655_formats[] = {
> +	{ MEDIA_BUS_FMT_RGB565_2X8_LE, V4L2_COLORSPACE_SRGB, 0x08},
> +};
> +
> static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
> {
> 	return &container_of(ctrl->handler, struct ov965x, ctrls.handler)->sd;
> @@ -894,12 +1272,16 @@ static int ov965x_set_test_pattern(struct ov965x *ov965x, int value)
> {
> 	int ret;
> 	u8 reg;
> +	u8 addr = (ov965x->id == OV9655V5_ID) ?
> +			REG_COM3 : REG_COM23;
> +	u8 mask = (ov965x->id == OV9655V5_ID) ?
> +			COM3_COLORBAR : COM23_TEST_MODE;
> 
> -	ret = ov965x_read(ov965x->client, REG_COM23, &reg);
> +	ret = ov965x_read(ov965x->client, addr, &reg);
> 	if (ret < 0)
> 		return ret;
> -	reg = value ? reg | COM23_TEST_MODE : reg & ~COM23_TEST_MODE;
> -	return ov965x_write(ov965x->client, REG_COM23, reg);
> +	reg = value ? reg | mask : reg & ~mask;
> +	return ov965x_write(ov965x->client, addr, reg);
> }
> 
> static int __g_volatile_ctrl(struct ov965x *ov965x, struct v4l2_ctrl *ctrl)
> @@ -1102,6 +1484,30 @@ static int ov965x_initialize_controls(struct ov965x *ov965x)
> 	return 0;
> }
> 
> +static int ov9655_initialize_controls(struct ov965x *ov965x)
> +{
> +	const struct v4l2_ctrl_ops *ops = &ov965x_ctrl_ops;
> +	struct ov965x_ctrls *ctrls = &ov965x->ctrls;
> +	struct v4l2_ctrl_handler *hdl = &ctrls->handler;
> +	int ret;
> +
> +	ret = v4l2_ctrl_handler_init(hdl, 16);
> +	if (ret < 0)
> +		return ret;
> +
> +	v4l2_ctrl_new_std_menu_items(hdl, ops, V4L2_CID_TEST_PATTERN,
> +				     ARRAY_SIZE(test_pattern_menu) - 1, 0, 0,
> +				     test_pattern_menu);
> +	if (hdl->error) {
> +		ret = hdl->error;
> +		v4l2_ctrl_handler_free(hdl);
> +		return ret;
> +	}
> +
> +	ov965x->sd.ctrl_handler = hdl;
> +	return 0;
> +}
> +
> /*
>  * V4L2 subdev video and pad level operations
>  */
> @@ -1518,9 +1924,15 @@ static int ov965x_detect_sensor(struct v4l2_subdev *sd)
> 
> 	if (!ret) {
> 		ov965x->id = OV965X_ID(pid, ver);
> -		if (ov965x->id == OV9650_ID || ov965x->id == OV9652_ID) {
> +		switch (ov965x->id) {
> +		case OV9650_ID:
> +		case OV9652_ID:
> 			v4l2_info(sd, "Found OV%04X sensor\n", ov965x->id);
> -		} else {
> +			break;
> +		case OV9655V5_ID:
> +			v4l2_info(sd, "Found OV%04X sensor\n", ov965x->id - 2);
> +			break;
> +		default:
> 			v4l2_err(sd, "Sensor detection failed (%04X, %d)\n",
> 				 ov965x->id, ret);
> 			ret = -ENODEV;
> @@ -1598,18 +2010,28 @@ static int ov965x_probe(struct i2c_client *client,
> 	if (ret < 0)
> 		goto err_me;
> 
> -	ov965x->init_regs = ov965x_init_regs;
> -	ov965x->initialize_controls = ov965x_initialize_controls;
> -	ov965x->framesizes = ov965x_framesizes;
> -	ov965x->nb_of_framesizes = ARRAY_SIZE(ov965x_framesizes);
> -	ov965x->formats = ov965x_formats;
> -	ov965x->nb_of_formats = ARRAY_SIZE(ov965x_formats);
> -	ov965x->intervals = ov965x_intervals;
> -	ov965x->nb_of_intervals = ARRAY_SIZE(ov965x_intervals);
> -	ov965x->fiv = &ov965x_intervals[0];
> -	ov965x->set_frame_interval = __ov965x_set_frame_interval;
> -	ov965x->update_exposure_ctrl = ov965x_update_exposure_ctrl;
> -	ov965x->set_params = __ov965x_set_params;
> +	if (ov965x->id != OV9655V5_ID) {
> +		ov965x->init_regs = ov965x_init_regs;
> +		ov965x->initialize_controls = ov965x_initialize_controls;
> +		ov965x->framesizes = ov965x_framesizes;
> +		ov965x->nb_of_framesizes = ARRAY_SIZE(ov965x_framesizes);
> +		ov965x->formats = ov965x_formats;
> +		ov965x->nb_of_formats = ARRAY_SIZE(ov965x_formats);
> +		ov965x->intervals = ov965x_intervals;
> +		ov965x->nb_of_intervals = ARRAY_SIZE(ov965x_intervals);
> +		ov965x->fiv = &ov965x_intervals[0];
> +		ov965x->set_frame_interval = __ov965x_set_frame_interval;
> +		ov965x->update_exposure_ctrl = ov965x_update_exposure_ctrl;
> +		ov965x->set_params = __ov965x_set_params;
> +	} else {
> +		ov965x->init_regs = ov9655_init_regs;
> +		ov965x->initialize_controls = ov9655_initialize_controls;
> +		ov965x->framesizes = ov9655_framesizes;
> +		ov965x->nb_of_framesizes = ARRAY_SIZE(ov9655_framesizes);
> +		ov965x->formats = ov9655_formats;
> +		ov965x->nb_of_formats = ARRAY_SIZE(ov9655_formats);
> +		ov965x->set_params = ov965x_set_frame_size;
> +	};
> 
> 	ov965x->frame_size = &ov965x->framesizes[0];
> 	ov965x_get_default_format(ov965x, &ov965x->format);
> @@ -1652,6 +2074,7 @@ static int ov965x_remove(struct i2c_client *client)
> static const struct i2c_device_id ov965x_id[] = {
> 	{ "OV9650", 0x9650 },
> 	{ "OV9652", 0x9652 },
> +	{ "OV9655", 0x9655 },

i2c device ids should be lower case to match compatible-strings in DT for
automatic modprobing.

Please pick/merge/copy&paste

<http://git.goldelico.com/?p=gta04-kernel.git;a=blobdiff;f=drivers/media/i2c/ov9650.c;h=c310cbee131665893d2d1df0ab1246bd9b1d41fe;hp=ed5d0a53a9c72036d6e017094b68111b5eb7f00d;hb=115b9c59202aa2fb1fecb691ebeef51220d363b8;hpb=da8ae2b038a448c8f822b3a4f20ed378db6d2934>

With this change I get:

root@letux:~# dmesg|fgrep ov96
[   12.727600] ov965x: Found OV9655 sensor
[   12.747711] ov965x 1-0030: ov965x driver probed
root@letux:~#

during probe.

> 	{ /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(i2c, ov965x_id);
> @@ -1659,6 +2082,7 @@ static int ov965x_remove(struct i2c_client *client)
> static const struct of_device_id ov965x_of_match[] = {
> 	{ .compatible = "ovti,ov9650", .data = (void *)0x9650 },
> 	{ .compatible = "ovti,ov9652", .data = (void *)0x9652 },
> +	{ .compatible = "ovti,ov9655", .data = (void *)0x9655 },
> 	{ /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, ov965x_of_match);
> -- 
> 1.9.1
>

[toc] | [prev] | [next] | [standalone]

#1672762

From	"H. Nikolaus Schaller" <hns@goldelico.com>
Date	2017-06-22 17:50 +0200
Message-ID	<tVcwO-6Is-5@gated-at.bofh.it>
In reply to	#1672724

> Am 22.06.2017 um 17:05 schrieb Hugues Fruchet <hugues.fruchet@st.com>:
> 
> This patchset enables OV9655 camera support.
> 
> OV9655 support has been tested using STM32F4DIS-CAM extension board
> plugged on connector P1 of STM32F746G-DISCO board.
> Due to lack of OV9650/52 hardware support, the modified related code
> could not have been checked for non-regression.
> 
> First patches upgrade current support of OV9650/52 to prepare then
> introduction of OV9655 variant patch.
> Because of OV9655 register set slightly different from OV9650/9652,
> not all of the driver features are supported (controls). Supported
> resolutions are limited to VGA, QVGA, QQVGA.
> Supported format is limited to RGB565.
> Controls are limited to color bar test pattern for test purpose.
> 
> OV9655 initial support is based on a driver written by H. Nikolaus Schaller [1].

Great!

I will test as soon as possible.

> OV9655 registers sequences come from STM32CubeF7 embedded software [2].

There is also a preliminary data sheet, e.g. here:

http://electricstuff.co.uk/OV9655-datasheet-annotated.pdf

> 
> [1] http://git.goldelico.com/?p=gta04-kernel.git;a=shortlog;h=refs/heads/work/hns/video/ov9655
> [2] https://developer.mbed.org/teams/ST/code/BSP_DISCO_F746NG/file/e1d9da7fe856/Drivers/BSP/Components/ov9655/ov9655.c
> 
> ===========
> = history =
> ===========
> version 1:
>  - Initial submission.
> 
> H. Nikolaus Schaller (1):
>  DT bindings: add bindings for ov965x camera module
> 
> Hugues Fruchet (5):
>  [media] ov9650: add device tree support
>  [media] ov9650: select the nearest higher resolution
>  [media] ov9650: use write_array() for resolution sequences
>  [media] ov9650: add multiple variant support
>  [media] ov9650: add support of OV9655 variant
> 
> .../devicetree/bindings/media/i2c/ov965x.txt       |  37 +
> drivers/media/i2c/Kconfig                          |   6 +-
> drivers/media/i2c/ov9650.c                         | 792 +++++++++++++++++----
> 3 files changed, 704 insertions(+), 131 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov965x.txt
> 
> -- 
> 1.9.1
> 

BR and thanks,
Nikolaus Schaller

[toc] | [prev] | [next] | [standalone]

#1673458

From	"H. Nikolaus Schaller" <hns@goldelico.com>
Date	2017-06-23 12:30 +0200
Message-ID	<tVu0F-Vv-1@gated-at.bofh.it>
In reply to	#1672762

Hi Hugues,

> Am 22.06.2017 um 17:41 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> 
>> Am 22.06.2017 um 17:05 schrieb Hugues Fruchet <hugues.fruchet@st.com>:
>> 
>> This patchset enables OV9655 camera support.
>> 
>> OV9655 support has been tested using STM32F4DIS-CAM extension board
>> plugged on connector P1 of STM32F746G-DISCO board.
>> Due to lack of OV9650/52 hardware support, the modified related code
>> could not have been checked for non-regression.
>> 
>> First patches upgrade current support of OV9650/52 to prepare then
>> introduction of OV9655 variant patch.
>> Because of OV9655 register set slightly different from OV9650/9652,
>> not all of the driver features are supported (controls). Supported
>> resolutions are limited to VGA, QVGA, QQVGA.
>> Supported format is limited to RGB565.
>> Controls are limited to color bar test pattern for test purpose.
>> 
>> OV9655 initial support is based on a driver written by H. Nikolaus Schaller [1].
> 
> Great!

Thanks again for picking up or work and trying to get it upstream.

> 
> I will test as soon as possible.

I have tried and had to fix some issues first:
* gpio properties have a different name than in our approach (but that is something maintainers have to decide and is easy to follow this or that way)
* there is no clock-frequency property which makes the driver request a clock frequency (something our camera interface expects this way)
* there is no vana-supply regulator and we need that to power on/off the camera on demand (reset and pwdn isn't enough in our hardware)
* for some unknown reason the driver does not load automatically from DT compatibility string and needs to be explicitly modprobed
* unfortunately we still get no image :(

The latter is likely a setup issue of our camera interface (OMAP3 ISP = Image Signal Processor) which
we were not yet able to solve. Oscilloscoping signals on the interface indicated that signals and
sync are correct. But we do not know since mplayer only shows a green screen.

Therefore we had not submitted anything upstream ourselves, because our driver setup
isn't finished and completely working.

I have written some more specific comments linked to proposals for patches as answer to your [PATCH v1 1/6]

BR and thanks,
Nikolaus

[toc] | [prev] | [next] | [standalone]

#1674193 — omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655 camera

From	Pavel Machek <pavel@ucw.cz>
Date	2017-06-25 11:20 +0200
Subject	omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655 camera
Message-ID	<tWbS1-3df-9@gated-at.bofh.it>
In reply to	#1673458

[Multipart message — attachments visible in raw view] — view raw

Hi!

> * unfortunately we still get no image :(
> 
> The latter is likely a setup issue of our camera interface (OMAP3 ISP = Image Signal Processor) which
> we were not yet able to solve. Oscilloscoping signals on the interface indicated that signals and
> sync are correct. But we do not know since mplayer only shows a green screen.

What mplayer command line do you use? How did you set up the pipeline
with media-ctl?

On kernel.org, I have tree called camera-fw5-6 , where camera works
for me on n900. On gitlab, there's modifed fcam-dev, which can be used
for testing.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1674447 — Re: omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655 camera

From	"H. Nikolaus Schaller" <hns@goldelico.com>
Date	2017-06-26 08:10 +0200
Subject	Re: omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655 camera
Message-ID	<tWvnH-76i-5@gated-at.bofh.it>
In reply to	#1674193

[Multipart message — attachments visible in raw view] — view raw

Hi Pavel,

> Am 25.06.2017 um 11:18 schrieb Pavel Machek <pavel@ucw.cz>:
> 
> Hi!
> 
>> * unfortunately we still get no image :(
>> 
>> The latter is likely a setup issue of our camera interface (OMAP3 ISP = Image Signal Processor) which
>> we were not yet able to solve. Oscilloscoping signals on the interface indicated that signals and
>> sync are correct. But we do not know since mplayer only shows a green screen.
> 
> What mplayer command line do you use? How did you set up the pipeline
> with media-ctl?
> 
> On kernel.org, I have tree called camera-fw5-6 , where camera works
> for me on n900. On gitlab, there's modifed fcam-dev, which can be used
> for testing.

We did have yet another (non-DT) camera driver and media-ctl working in with 3.12.37,
but had no success yet to update it to work with modern kernels or drivers. It
is either that the (newer) drivers missing something or the media-ctl has changed.

Here is the log of our scripts with Hugues' driver and our latest setup:

root@letux:~# ./camera-demo sxga
DISPLAY=:0
XAUTHORITY=tcp
Camera: /dev/v4l-subdev8
Setting mode sxga
media-ctl -r
media-ctl -l '"ov965x":0 -> "OMAP3 ISP CCDC":0[1]'
media-ctl -l '"OMAP3 ISP CCDC":1 -> "OMAP3 ISP CCDC output":0[1]'
media-ctl -V '"ov965x":0 [UYVY2X8 1280x1024]'
media-ctl -V '"OMAP3 ISP CCDC":0 [UYVY2X8 1280x1024]'
media-ctl -V '"OMAP3 ISP CCDC":1 [UYVY 1280x1024]'
### starting mplayer in sxga mode ###
mplayer tv:// -vf rotate=2 -tv driver=v4l2:device=/dev/video2:outfmt=uyvy:width=1280:height=1024:fps=15 -vo x11
MPlayer2 2.0-728-g2c378c7-4+b1 (C) 2000-2012 MPlayer Team

Playing tv://.
Detected file format: TV
Selected driver: v4l2
 name: Video 4 Linux 2 input
 author: Martin Olschewski <olschewski@zpr.uni-koeln.de>
 comment: first try, more to come ;-)
v4l2: ioctl get standard failed: Invalid argument
Selected device: OMAP3 ISP CCDC output
 Capabilities:  video capture  video output  streaming
 supported norms:
 inputs: 0 = camera;
 Current input: 0
 Current format: unknown (0x0)
tv.c: norm_from_string(pal): Bogus norm parameter, setting default.
v4l2: ioctl enum norm failed: Inappropriate ioctl for device
Error: Cannot set norm!
Selected input hasn't got a tuner!
v4l2: ioctl set mute failed: Inappropriate ioctl for device
v4l2: ioctl query control failed: Inappropriate ioctl for device
v4l2: ioctl query control failed: Inappropriate ioctl for device
v4l2: ioctl query control failed: Inappropriate ioctl for device
v4l2: ioctl query control failed: Inappropriate ioctl for device
v4l2: ioctl streamon failed: Broken pipe
[ass] auto-open
Opening video filter: [rotate=2]
VIDEO:  1280x1024  15.000 fps    0.0 kbps ( 0.0 kB/s)
Could not find matching colorspace - retrying with -vf scale...
Opening video filter: [scale]
[swscaler @ 0xb5ca9980]using unscaled uyvy422 -> yuv420p special converter
VO: [x11] 1024x1280 => 1024x1280 Planar YV12
[swscaler @ 0xb5ca9980]No accelerated colorspace conversion found from yuv420p to bgra.
Colorspace details not fully supported by selected vo.
Selected video codec: RAW UYVY [raw]
Audio: no sound
Starting playback...
V:   0.0  10/ 10 ??% ??% ??,?% 0 0 $<3>


MPlayer interrupted by signal 2 in module: filter_video
V:   0.0  11/ 11 ??% ??% ??,?% 0 0 $<3>
v4l2: ioctl set mute failed: Inappropriate ioctl for device
v4l2: 0 frames successfully processed, 0 frames dropped.

Exiting... (Quit)
root@letux:~#

BR and thanks,
Nikolaus

[toc] | [prev] | [next] | [standalone]

Page 2 of 3 — ← Prev page 1 [2] 3 Next page →

csiph-web

[PATCH v1 0/6] Add support of OV9655 camera

Contents

#1676619 — Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module

#1675017 — Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module

#1675013 — Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module

#1672728 — [PATCH v1 2/6] [media] ov9650: add device tree support

#1674927 — Re: [PATCH v1 2/6] [media] ov9650: add device tree support

#1674976 — Re: [PATCH v1 2/6] [media] ov9650: add device tree support

#1675304 — Re: [PATCH v1 2/6] [media] ov9650: add device tree support

#1675509 — Re: [PATCH v1 2/6] [media] ov9650: add device tree support

#1672729 — [PATCH v1 4/6] [media] ov9650: use write_array() for resolution sequences

#1674930 — Re: [PATCH v1 4/6] [media] ov9650: use write_array() for resolution sequences

#1677781 — Re: [PATCH v1 4/6] [media] ov9650: use write_array() for resolution sequences

#1672732 — [PATCH v1 3/6] [media] ov9650: select the nearest higher resolution

#1672734 — [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant

#1674246 — Re: [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant

#1674247 — [PATCH] ov9650: fix semicolon.cocci warnings

#1674448 — Re: [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant

#1672762

#1673458

#1674193 — omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655 camera

#1674447 — Re: omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655 camera