Re: [PATCH v2 2/2] media: ov9650: add device tree binding

[Akinobu Mita]

2018-01-12 23:15 GMT+09:00 Sylwester Nawrocki :
> On 01/08/2018 10:35 AM, Sakari Ailus wrote:
>
>> I was going to say you're missing the MAINTAINERS entry for this newly
>> added file but then I noticed that the entire driver is missing an entry.
>> Still this file should have a MAINTAINERS entry added for it independently
>> of that, in the same patch.
>>
>> Cc Sylwester.
>
> I don't the hardware and I can't test the patches so Mita-san if you wish
> so please add yourself as a maintainer of whole driver.

Even if you don't have the hardware, you can help reviwing.  So if you
don't mind, I would like to add the following maintainer entry for this
driver.

OMNIVISION OV9650 SENSOR DRIVER
M:  Sakari Ailus 
R:  Akinobu Mita 
R:  Sylwester Nawrocki 
L:  linux-media@vger.kernel.org
T:  git git://linuxtv.org/media_tree.git
S:  Maintained
F:  drivers/media/i2c/ov9650.c
F:  Documentation/devicetree/bindings/media/i2c/ov9650.txt

Re: [PATCH v2 2/2] media: ov9650: add device tree binding

[Sylwester Nawrocki]

On 01/08/2018 10:35 AM, Sakari Ailus wrote:
 
> I was going to say you're missing the MAINTAINERS entry for this newly
> added file but then I noticed that the entire driver is missing an entry.
> Still this file should have a MAINTAINERS entry added for it independently
> of that, in the same patch.
> 
> Cc Sylwester.

I don't the hardware and I can't test the patches so Mita-san if you wish
so please add yourself as a maintainer of whole driver.

-- 
Regards,
Sylwester

Re: [PATCH v2 2/2] media: ov9650: add device tree binding

[Rob Herring]

On Mon, Jan 08, 2018 at 01:54:24AM +0900, Akinobu Mita wrote:
> Now the ov9650 driver supports device tree probing.  So this adds a
> device tree binding documentation.
> 
> Cc: Jacopo Mondi 
> Cc: H. Nikolaus Schaller 
> Cc: Hugues Fruchet 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Cc: Rob Herring 
> Signed-off-by: Akinobu Mita 
> ---
> * Changelog v2
> - Split binding documentation, suggested by Rob Herring and Jacopo Mondi
> - Improve the wording for compatible property in the binding documentation,
>   suggested by Jacopo Mondi
> - Improve the description for the device node in the binding documentation,
>   suggested by Sakari Ailus
> 
>  .../devicetree/bindings/media/i2c/ov9650.txt   | 36 
> ++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt

Reviewed-by: Rob Herring 

Re: [PATCH v2 2/2] media: ov9650: add device tree binding

[Sakari Ailus]

Hi Akinobu,

Thanks for the update.

On Mon, Jan 08, 2018 at 01:54:24AM +0900, Akinobu Mita wrote:
> Now the ov9650 driver supports device tree probing.  So this adds a
> device tree binding documentation.
> 
> Cc: Jacopo Mondi 
> Cc: H. Nikolaus Schaller 
> Cc: Hugues Fruchet 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Cc: Rob Herring 
> Signed-off-by: Akinobu Mita 
> ---
> * Changelog v2
> - Split binding documentation, suggested by Rob Herring and Jacopo Mondi
> - Improve the wording for compatible property in the binding documentation,
>   suggested by Jacopo Mondi
> - Improve the description for the device node in the binding documentation,
>   suggested by Sakari Ailus
> 
>  .../devicetree/bindings/media/i2c/ov9650.txt   | 36 
> ++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov9650.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
> new file mode 100644
> index 000..506dfc5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
> @@ -0,0 +1,36 @@
> +* Omnivision OV9650/OV9652 CMOS sensor
> +
> +Required Properties:
> +- compatible: shall be one of
> + "ovti,ov9650"
> + "ovti,ov9652"
> +- clocks: reference to the xvclk input clock.
> +
> +Optional Properties:
> +- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
> +  Active is high.
> +- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
> +  Active is high.
> +
> +The device node shall contain one 'port' child node with one child 'endpoint'
> +subnode for its digital output video port, in accordance with the video
> +interface bindings defined in Documentation/devicetree/bindings/media/
> +video-interfaces.txt.
> +
> +Example:
> +
> + {
> + ov9650: camera@30 {
> + compatible = "ovti,ov9650";
> + reg = <0x30>;
> + reset-gpios = <_gpio_0 0 GPIO_ACTIVE_HIGH>;
> + powerdown-gpios = <_gpio_0 1 GPIO_ACTIVE_HIGH>;
> + clocks = <>;
> +
> + port {
> + ov9650_0: endpoint {
> + remote-endpoint = <_in0>;
> + };
> + };
> + };
> +};

I was going to say you're missing the MAINTAINERS entry for this newly
added file but then I noticed that the entire driver is missing an entry.
Still this file should have a MAINTAINERS entry added for it independently
of that, in the same patch.

Cc Sylwester.

-- 
Kind regards,

Sakari Ailus
sakari.ai...@linux.intel.com

[PATCH v2 2/2] media: ov9650: add device tree binding

[Akinobu Mita]

Now the ov9650 driver supports device tree probing.  So this adds a
device tree binding documentation.

Cc: Jacopo Mondi 
Cc: H. Nikolaus Schaller 
Cc: Hugues Fruchet 
Cc: Sakari Ailus 
Cc: Mauro Carvalho Chehab 
Cc: Rob Herring 
Signed-off-by: Akinobu Mita 
---
* Changelog v2
- Split binding documentation, suggested by Rob Herring and Jacopo Mondi
- Improve the wording for compatible property in the binding documentation,
  suggested by Jacopo Mondi
- Improve the description for the device node in the binding documentation,
  suggested by Sakari Ailus

 .../devicetree/bindings/media/i2c/ov9650.txt   | 36 ++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov9650.txt 
b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
new file mode 100644
index 000..506dfc5
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
@@ -0,0 +1,36 @@
+* Omnivision OV9650/OV9652 CMOS sensor
+
+Required Properties:
+- compatible: shall be one of
+   "ovti,ov9650"
+   "ovti,ov9652"
+- clocks: reference to the xvclk input clock.
+
+Optional Properties:
+- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
+  Active is high.
+- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
+  Active is high.
+
+The device node shall contain one 'port' child node with one child 'endpoint'
+subnode for its digital output video port, in accordance with the video
+interface bindings defined in Documentation/devicetree/bindings/media/
+video-interfaces.txt.
+
+Example:
+
+ {
+   ov9650: camera@30 {
+   compatible = "ovti,ov9650";
+   reg = <0x30>;
+   reset-gpios = <_gpio_0 0 GPIO_ACTIVE_HIGH>;
+   powerdown-gpios = <_gpio_0 1 GPIO_ACTIVE_HIGH>;
+   clocks = <>;
+
+   port {
+   ov9650_0: endpoint {
+   remote-endpoint = <_in0>;
+   };
+   };
+   };
+};
-- 
2.7.4