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?