Hi Vladimir, On Tue, Mar 27, 2018 at 12:37:31PM +0300, Vladimir Zapolskiy wrote: > Hi Jacopo, > > On 03/27/2018 11:57 AM, jacopo mondi wrote: > > Hi Vladimir, > > > > On Tue, Mar 27, 2018 at 11:30:29AM +0300, Vladimir Zapolskiy wrote: > >> Hi Sergei, > >> > >> On 03/27/2018 11:27 AM, Sergei Shtylyov wrote: > >>> 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? > >>> > >> > >> Seconded, with a correction to "thine,oe-gpios". > >> > > > > mmm, okay then... > > > > A grep for that semi-standard properties names in Documentation/ > > returns only usage examples and no actual definitions, so I assume this > > is why they are semi-standard. > > Here we have to be specific about a particular property, let it be 'oe-gpios' > vs. 'enable-gpios' and let's collect some statistics: > > % grep -Hr oe-gpios Documentation/devicetree/bindings/* | wc -l > 0 > > $ grep -Hr enable-gpios Documentation/devicetree/bindings/* | wc -l > 86 > > While 'thine,oe-gpios' would be correct, I see no reason to introduce a vendor > specific property to define a pin with a common and well understood purpose. > > If you go forward with the vendor specific prefix, apparently you can set the name > to 'thine,oe-gpio' (single) or even to 'thine,oe', or does the datasheet names > the pin as "OE GPIO" or "OE connected to a GPIO"? I guess no. > Let me clarify I don't want to push for a vendor specific name or similar, I'm fine with using 'semi-standard' names, I'm just confused by the 'semi-standard' definition. I guess from your examples, the usage count makes a difference here. > Standards do not define '-gpios' suffix, but partially the description is found > in Documentation/bindings/gpio/gpio.txt, still it is not a section in any > standard as far as I know. > > > Seems like there is some tribal knowledge involved in defining what > > is semi-standard and what's not, or are those properties documented somewhere? > > > > The point is that there is no formal standard which describes every IP, > every IC and every single their property, some device node names and property > names are recommended in ePAPR and Devicetree Specification though. > > Think of a confusion if 'rst-gpios' (have you seen any ICs with an RST pin?) and > 'reset-gpios' are different. Same applies to 'pdwn-gpios' vs. 'powerdown-gpios'. I see all your points and I agree with most of them. Anyway, if the chip manual describes a pin as 'RST' I would not find it confusing to have a 'rst-gpio' defined in bindings :) Let me be a bit pesky here: what if a chip defines a reset GPIO, which is definitely a reset, but names it, say "XYZ" ? Would you prefer to see it defined as "reset-gpios" for consistency with other bindings, or "xyz-gpios" for consistency with documentation? Thanks j > > >> If vendor agnostic properties are supposed to be used, then please follow > >> the referenced by Rob semi-standard notations. > > -- > With best wishes, > Vladimir
Attachment:
signature.asc
Description: PGP signature
