On Thu, Apr 5, 2018 at 1:53 PM, Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > Hi Rob, > > On Thursday, 5 April 2018 19:33:33 EEST Rob Herring wrote: >> On Mon, Apr 2, 2018 at 8:36 AM, Laurent Pinchart wrote: >> > On Tuesday, 27 March 2018 14:03:25 EEST Vladimir Zapolskiy wrote: >> >> On 03/27/2018 01:10 PM, jacopo mondi wrote: >> >>> On Tue, Mar 27, 2018 at 12:37:31PM +0300, Vladimir Zapolskiy wrote: >> >>>> On 03/27/2018 11:57 AM, jacopo mondi wrote: >> >>>>> On Tue, Mar 27, 2018 at 11:30:29AM +0300, Vladimir Zapolskiy wrote: >> >>>>>> On 03/27/2018 11:27 AM, Sergei Shtylyov wrote: >> >>>>>>> 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,thc63l >> >>>>>>>>>>>>> vd1024.txt >> >>>>>>>>>>>>> diff --git >> >>>>>>>>>>>>> a/Documentation/devicetree/bindings/display/bridge/thine,thc6 >> >>>>>>>>>>>>> 3lvd1024.txt >> >>>>>>>>>>>>> b/Documentation/devicetree/bindings/display/bridge/thine,thc6 >> >>>>>>>>>>>>> 3lvd1024.txt >> >>>>>>>>>>>>> new file mode 100644 >> >>>>>>>>>>>>> index 0000000..8225c6a >> >>>>>>>>>>>>> --- /dev/null >> >>>>>>>>>>>>> +++ >> >>>>>>>>>>>>> b/Documentation/devicetree/bindings/display/bridge/thine,thc6 >> >>>>>>>>>>>>> 3lvd1024.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. >> > >> > I have to disagree with this. When using a property name that doesn't >> > correspond to the hardware documentation, developers will need to refer to >> > the DT bindings documentation to confirm the property name. "Widely used" >> > property names will not save time, they will use more time. This is of >> > course marginal and I don't think it would have any noticeable impact, >> > but I don't think your argument holds. >> >> We can have it both ways. The name should follow the documented >> name/function. For example, we have enable-gpios which is simply the >> invert of powerdown-gpios (for software's purposes). Pick the one >> closest to the documentation. We're not trying to make bindings use >> "enable" if a signal is called "powerdown". >> >> What we don't want is gratuitous variation in the names based on the >> whims of hw designers: >> >> resetb-gpios >> resetn-gpios >> rst-gpios >> rstn-gpios >> nRESET-gpios >> >> ...you get the idea (and I left out vendor prefixes). > > Do we have a list of standardized names that should be used preferentially ? No. I think the list is pretty much in this thread. I didn't find any others grepping the tree. There was an effort with USB devices to do "generic" power/reset control, but I think that never landed. It was using standard property names like reset-gpios within the device nodes rather than the approach used by mmc pwrseq binding (which I'm not a fan of). > If not, should we create one ? Sure. You can put it in the root. Then maybe people will find it. >> > I'm all for standardizing properties across DT bindings for multiple >> > components, but doing so in a semi-random fashion will in my opinion not >> > result in any gain. We can decide that power-down or output-enable GPIOS >> > should have common property names (and I'm not even sure that would be >> > useful, but we can certainly discuss it), but in that case someone should >> > make a proposal and get the names standardized. Unless we do so, no >> > matter what property name gets picked for a particular binding, it won't >> > become universally used by magic. >> >> For "output enable", I suspect that is a common signal/function and >> should have a standardized name. Generally, the way this works is we >> get several variations and then we try to standardize things. I think >> we can all agree standardizing first is better. If you want to put it >> in a common place, please do. Maybe people will read that. Regardless, >> the only way to enforce following standard names is with review. >> >> Debating "oe" vs. "output-enable" is bikeshedding IMO. Anyone familiar >> with h/w design should recognize OE. >> >> The reason to try and standardize names is so we can have common >> drivers or library functions. In particular, for things like GPIOs >> that need to be configured first for devices on otherwise discoverable >> buses, this is very useful. > > I'm not sure we will ever implement that for the OE or power-down GPIOs, but > I'm also not sure we will never do it, so I suppose it makes sense, just in > case. It's no different than "power-supply" in the simple panel binding which we support in the common driver. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
