2018-04-09 18:06 GMT+09:00 jacopo mondi <jac...@jmondi.org>:
> Hi Akinobu,
>
> On Sun, Apr 08, 2018 at 12:48:09AM +0900, Akinobu Mita wrote:
>> This adds a device tree binding documentation for OV7720/OV7725 sensor.
>
> Please use as patch subject
> media: dt-bindings:

OK.

>>
>> Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
>> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
>> Cc: Hans Verkuil <hans.verk...@cisco.com>
>> Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
>> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
>> Cc: Rob Herring <robh...@kernel.org>
>> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
>> ---
>>  .../devicetree/bindings/media/i2c/ov772x.txt       | 36 
>> ++++++++++++++++++++++
>>  MAINTAINERS                                        |  1 +
>>  2 files changed, 37 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt 
>> b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
>> new file mode 100644
>> index 0000000..9b0df3b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
>> @@ -0,0 +1,36 @@
>> +* Omnivision OV7720/OV7725 CMOS sensor
>> +
>
> Could you please provide a brief description of the sensor (supported
> resolution and formats is ok)

OK.

>> +Required Properties:
>> +- compatible: shall be one of
>> +     "ovti,ov7720"
>> +     "ovti,ov7725"
>> +- clocks: reference to the xclk input clock.
>> +- clock-names: shall be "xclk".
>> +
>> +Optional Properties:
>> +- rstb-gpios: reference to the GPIO connected to the RSTB pin, if any.
>> +- pwdn-gpios: reference to the GPIO connected to the PWDN pin, if any.
>
> As a general note:
> This is debated, and I'm not enforcing it, but please consider using
> generic names for GPIOs with common functions. In this case
> "reset-gpios" and "powerdown-gpios". Also please indicate the GPIO
> active level in bindings description.
>
> For this specific driver:
> The probe routine already looks for a GPIO named 'pwdn', so I guess
> the DT bindings should use the same name. Unless you're willing to
> change it in the board files that register it (Migo-R only in mainline) and
> use the generic 'powerdown' name for both. Either is fine with me.

I'll prepare anothre patch that renames the GPIO names to generic one in
this driver and Mingo-R board file.

> There is no support for the reset GPIO in the driver code, it
> supports soft reset only. Either ditch it from bindings or add support
> for it in driver's code.

Doesn't that reset GPIO exist in current ov772x driver code, does it?

Reply via email to