Hi Rob, sorry for pointing to you directly :) On Mon, Apr 02, 2018 at 04:36:55PM +0300, Laurent Pinchart wrote: > Hi Vladimir, > > 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: [snip] > > >>>>>>>>>> > > >>>>>>>>>>> +- 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. > > 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. > > I'd like to hear the DT bindings maintainers position on this matter. > Me too :) As driver developer I see both Vladimir's and Laurent's points. I still prefer to reflect in bindings the pin name assigned in the chip manual, over semi-standard names, but that's a personal preference. In order to send next version I would like to know which direction do the dt custodians should be taken on this. Thanks j > -- > Regards, > > Laurent Pinchart > > >
Attachment:
signature.asc
Description: PGP signature