Hi Vladimir, On Tue, Mar 27, 2018 at 02:03:25PM +0300, Vladimir Zapolskiy wrote: > Hi Jacopo, > > On 03/27/2018 01:10 PM, jacopo mondi wrote: > > 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. > > yes, in gneneral you can read "semi-standard" as "widely used", thus collecting > statistics is a good enough method to make a reasoning. > > Hopefully the next evolutionary step of "widely used" is "described in standard". > > >> 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? > > If a pin is definitely an IC reset as you said, then my preference is to see > it described under 'reset-gpios' property name, plus a comment in the IC > device tree documentation document about it. I can provide two reasons to > advocate my position: > > 1) developers spend significantly more time reading and editing the actual > DTSI/DTS board files rather than reading and editing documentation, > it makes sense to use common property names to save time and reduce > amount of "what does 'oe' stand for?" type of questions; I suppose > that the recommendation to avoid not "widely used" abbreviations in > device node and property names arises from the same reasoning, > > 2) "widely used" and "standard" properties are excellent candidates for > developing (or re-using) generalization wrappers, it happened so many > times in the past, and this process shall be supported in my opinion; > due to compatibility restrictions it might be problematic to change > property names, and every new exception to "widely used" properties > makes problematic to develop and maintain these kinds of wrappers, and > of course it postpones a desired "described in standard" recognition. > > If my point of view is accepted, I do admit that a developer who > translates a board schematics to board DTS file may experience a minor > discomfort, which is mitigated if relevant pin names are found in device > tree binding documentation in comments to properties, still the overall > gain is noticeably higher in my personal opinion. > Thank you for sharing your point of view. Makes much sense actually. I will use semi-standard names in v7 bindings. Thanks j > -- > With best wishes, > Vladimir
Attachment:
signature.asc
Description: PGP signature
