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!
