Re: [PATCH v6 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[Sergei Shtylyov]

Hello!

On 3/27/2018 10:33 AM, jacopo mondi wrote:
[...]
> > > > > > Document Thine THC63LVD1024 LVDS decoder device tree bindings.
> > > > > >
> > > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> > > > > > Reviewed-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
> > > > > > Reviewed-by: Niklas SÃderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > > > > > ---
> > > > > >   .../bindings/display/bridge/thine,thc63lvd1024.txt | 66 +++++++++++++++++++
> > > > > >   1 file changed, 66 insertions(+)
> > > > > >   create mode 100644
> > > > > > Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > > > > >
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > > > > > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > > > > > new file mode 100644
> > > > > > index 0000000..8225c6a
> > > > > > --- /dev/null
> > > > > > +++
> > > > > > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > > > > > @@ -0,0 +1,66 @@
> > > > > > +Thine Electronics THC63LVD1024 LVDS decoder
> > > > > > +-------------------------------------------
> > > > > > +
> > > > > > +The THC63LVD1024 is a dual link LVDS receiver designed to convert LVDS
> > > > > > streams
> > > > > > +to parallel data outputs. The chip supports single/dual input/output modes,
> > > > > > +handling up to two two input LVDS stream and up to two digital CMOS/TTL
> > > > > > outputs.
> > > > > > +
> > > > > > +Single or dual operation modes, output data mapping and DDR output modes
> > > > > > are
> > > > > > +configured through input signals and the chip does not expose any control
> > > > > > bus.
> > > > > > +
> > > > > > +Required properties:
> > > > > > +- compatible: Shall be "thine,thc63lvd1024"
> > > > > > +
> > > > > > +Optional properties:
> > > > > > +- vcc-supply: Power supply for TTL output and digital circuitry
> > > > > > +- cvcc-supply: Power supply for TTL CLOCKOUT signal
> > > > > > +- lvcc-supply: Power supply for LVDS inputs
> > > > > > +- pvcc-supply: Power supply for PLL circuitry
> > > > > As explained in a comment to one of the previous versions of this series, I'm
> > > > > tempted to make vcc-supply mandatory and drop the three other power supplies
> > > > > for now, as I believe there's very little chance they will be connected to
> > > > > separately controllable regulators (all supplies use the same voltage). In the
> > > > > very unlikely event that this occurs in design we need to support in the
> > > > > future, the cvcc, lvcc and pvcc supplies can be added later as optional
> > > > > without breaking backward compatibility.
> > > > I'm okay with that.
> > > >
> > > > > Apart from that,
> > > > >
> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > > > >
> > > > > > +- pdwn-gpios: Power down GPIO signal. Active low
> > > > powerdown-gpios is the semi-standard name.
> > > right, I've also noticed it. If possible please avoid shortenings in
> > > property names.
> >
> > It is not shortening, it just follow pin name from decoder's datasheet.
> >
> > > > > > +- oe-gpios: Output enable GPIO signal. Active high
> > > > > > +
> > > And this one is also a not ever met property name, please consider to
> > > rename it to 'enable-gpios', for instance display panels define it.
> >
> >
> > Again, it follows datasheet naming scheme. Has something changed in DT
> > conventions?
>
> Seconded. My understanding is that the property name should reflect
> what reported in the the chip manual. For THC63LVD1024 the enable and
> power down pins are named 'OE' and 'PDWN' respectively.

   But don't we need the vendor prefix in the prop names then, like 
"renesas,oe-gpios" then?

> Thanks
>     j

MBR, Sergei