Messages in this thread | | | Date | Wed, 29 Nov 2023 12:37:26 +0100 | From | Dragan Simic <> | Subject | Re: [PATCH v3] docs: dt-bindings: add DTS Coding Style document |
| |
On 2023-11-29 11:43, Krzysztof Kozlowski wrote: > On 28/11/2023 21:00, Dragan Simic wrote: >> >> I went through the language of the entire patch, after the notice that >> the v4 would no longer accept language improvements. My wording- and >> grammar-related suggestions are available inline below. > > Thanks. I want to finish this at some point and it might not happen if > grammar fixes will be coming every patch revision. Then after we finish > review, new feedback will appear about using British or American > spelling (which reminds me old quote/email about which variant of > English is most popular in Linux kernel: the incorrect one).
Ah, that's a good one. :) Basically, both English variants should be fine, but a single document should obviously use only one variant.
>>> +===================================== >>> +Devicetree Sources (DTS) Coding Style >>> +===================================== >>> + >>> +When writing Devicetree Sources (DTS) please observe below >>> guidelines. >>> They >> >> The sentence above should be replaced with: "The following guidelines >> are to be followed when writing Devicetree Source (DTS) files." > > Are you sure? It's passive and I was taught it is discouraged for > writing. See for example: > https://www.hamilton.edu/academics/centers/writing/seven-sins-of-writing/1
Hmm, you're right, passive voice is usually not the best choice. Here's my take two for the suggested replacement sentence, which is actually a simplified version:
"This document contains the guidelines for writing Devicetree Source (DTS) files."
>>> +should be considered complementary to any rules expressed already in >>> Devicetree >>> +Specification and dtc compiler (including W=1 and W=2 builds). >> >> A definite article ("the") should be added before "Devicetree > > ack > >> Specification" and "dtc". Also, "Specification" in "Devicetree >> Specification" should be capitalized. > > It was.
Oh, sorry, I see now. IIRC, it wasn't capitalized in some places, so I made a mistake here.
>>> + >>> +Individual architectures and sub-architectures can add additional >>> rules, making >>> +the style stricter. >> >> "Sub-architectures" should be replaced with "subarchitectures". "Can > > A hint, you can write such review feedback as: > s/sub-architectures/subarchitectures/
Sure, but I specifically wanted to be less terse, as a way to be respectful.
> BTW, my language spelling points "subarchitectures" as mistake, but > sure, ack.
Using hyphens or not is almost always debatable, but modern English in general leans toward not using them.
>>> +3. Unit addresses shall use lowercase hex, without leading zeros >>> (padding). >> >> "Lowercase hex" should be replaced with "lowercase hexadecimal >> digits". >> >>> + >>> +4. Hex values in properties, e.g. "reg", shall use lowercase hex. >>> The >>> address >>> + part can be padded with leading zeros. >> >> "Hex values" should be replaced with "Hexadecimal values". "Lowercase >> hex" should be replaced with "lowercase hexadecimal digits". > > ack, but that's quite picky. We are (software) engineers so we are > supposed to know the slang.
Sure, but this document is of a bit formal nature, so using slightly more formal language can only be helpful.
>>> +2. Nodes without unit addresses shall be ordered alpha-numerically >>> by >>> the node >>> + name. For a few types of nodes, they can be ordered by the main >>> property >>> + (e.g. pin configuration states ordered by value of "pins" >>> property). >> >> "Alpha-numerically" should be replaced with "alphabetically". > > Are you sure? Does alphabetical order include numbers?
That's a good question, which also crossed my mind while writing the suggestions down. A more correct word would be "lexicographically", with something like ", with the already defined valid characters making the symbol set and the ACSII character set defining the ordering, " serving as an additional explanation.
This would be a rather formal, but also very precise definition of the applied ordering.
>>> +3. When extending nodes in the board DTS via &label, the entries >>> shall >>> be >>> + ordered either alpha-numerically or by keeping the order from >>> DTSI >>> (choice >>> + depending on sub-architecture). >> >> "Alpha-numerically" should be replaced with "alphabetically". > > Similar concern
I agree. We could use "lexicographically" instead, with the precise definition already established earlier in the document.
>>> +board DTS, not in the SoC or SoM DTSI. A partial exception is a >>> common >>> +external reference SoC-input clock, which could be coded as a >>> fixed-clock in >> >> "SoC-input" should be replaced with "SoC input". > > ack, thanks!
Thank you once again for working on this document!
| |