Re: [PATCH v3] docs: dt-bindings: add DTS Coding Style document

From: Dragan Simic
Date: Wed Nov 29 2023 - 06:37:38 EST


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!