Re: [PATCH v9 23/24] dt: bindings: smiapp: Document lens-focus and flash properties

Sakari Ailus
2017-09-11T07:45:50Z

On Fri, Sep 08, 2017 at 11:45:44PM +0200, Pavel Machek wrote:
> On Fri 2017-09-08 16:42:26, Sakari Ailus wrote:
> > Hi Pavel,
> >
> > On Fri, Sep 08, 2017 at 03:36:52PM +0200, Pavel Machek wrote:
> > > Hi!
> > >
> > > > Document optional lens-focus and flash properties for the smiapp driver.
> > > >
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > ---
> > > > Documentation/devicetree/bindings/media/i2c/nokia,smia.txt | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> > > > index 855e1faf73e2..a052969365d9 100644
> > > > --- a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> > > > +++ b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> > > > @@ -27,6 +27,8 @@ Optional properties
> > > > - nokia,nvm-size: The size of the NVM, in bytes. If the size is not given,
> > > > the NVM contents will not be read.
> > > > - reset-gpios: XSHUTDOWN GPIO
> > > > +- flash-leds: One or more phandles to refer to flash LEDs
> > > > +- lens-focus: Phandle for lens focus
> > >
> > > Should we simply reference the generic documentation here? If it needs
> > > changing, it will be easier changing single place.
> >
> > Good question.
> >
> > Ideally the properties at least would never change; we do have a common
> > parser for the properties which is part of the patchset so in theory it
> > would be possible to change the documentation in a backward-compatible way.
> >
> > I added these properties as well as all the other properties supported by
> > the sensor driver are documented here. That has been the practice AFAIU,
> > albeit omissions do happen occasionally.
> >
> > The issue I see with omitting the documentation here is that the user
> > otherwise won't know whether a driver uses a given property or not: it
> > shouldn't be necessary to read driver code to write DT source.
>
> I see... OTOH documentation in central place _is_ a bit more verbose:
>
> +- flash-leds: An array of phandles, each referring to a flash LED, a sub-node
> + of the LED driver device node.
>
> What about:
>
> flash-leds, lens-focus: see <reference to central place>
>
> ?

How about this?

diff --git a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
index a052969365d9..33f10a94c381 100644
--- a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
+++ b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
@@ -27,8 +27,8 @@ Optional properties
- nokia,nvm-size: The size of the NVM, in bytes. If the size is not given,
the NVM contents will not be read.
- reset-gpios: XSHUTDOWN GPIO
-- flash-leds: One or more phandles to refer to flash LEDs
-- lens-focus: Phandle for lens focus
+- flash-leds: See ../video-interfaces.txt
+- lens-focus: See ../video-interfaces.txt

Endpoint node mandatory properties

I prefer to put them on separate lines as it's easier to read that way.

--
Sakari Ailus
e-mail: sakari.ailus@iki.fi