Re: [PATCH 5/6] media: ov772x: add device tree binding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2018-04-09 18:06 GMT+09:00 jacopo mondi <jacopo@xxxxxxxxxx>:
> 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+renesas@xxxxxxxxxx>
>> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>> Cc: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>> Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
>> Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>
>> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
>> Signed-off-by: Akinobu Mita <akinobu.mita@xxxxxxxxx>
>> ---
>>  .../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?



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux