On 25/11/2023 20:47, Andrew Lunn wrote:
>> +=====================================
>> +Devicetree Sources (DTS) Coding Style
>> +=====================================
>> +
>> +When writing Devicetree Sources (DTS) please observe below guidelines. They
>> +should be considered complementary to any rules expressed already in Devicetree
>> +Specification and dtc compiler (including W=1 and W=2 builds).
>> +
>> +Individual architectures and sub-architectures can add additional rules, making
>> +the style stricter.
>
> It would be nice to add a pointer where such rules are documented.
Subsystem profile or any other place. The generic doc should not point
to specific ones.
>
>> +Naming and Valid Characters
>> +---------------------------
>> +
>> +Devicetree specification allows broader range of characters in node and
>> +property names, but for code readability the choice shall be narrowed.
>> +
>> +1. Node and property names are allowed to use only:
>> +
>> + * lowercase characters: [a-z]
>> + * digits: [0-9]
>> + * dash: -
>> +
>> +2. Labels are allowed to use only:
>> +
>> + * lowercase characters: [a-z]
>> + * digits: [0-9]
>> + * underscore: _
>> +
>> +3. Unit addresses shall use lowercase hex, without leading zeros (padding).
>> +
>> +4. Hex values in properties, e.g. "reg", shall use lowercase hex. The address
>> + part can be padded with leading zeros.
>> +
>> +Example::
>> +
>> + gpi_dma2: dma-controller@800000 {
>
> Not the best of example. Upper case 8 does not exist, as far as i
> known.
Sure, this was taken from DTS, but I can bring here some fake address to
illustrate :)
>
>> + compatible = "qcom,sm8550-gpi-dma", "qcom,sm6350-gpi-dma";
>> + reg = <0x0 0x00800000 0x0 0x60000>;
>
> Maybe introduce some [a-f] in the example reg?
>
>> +Order of Nodes
>> +--------------
>> +
>> +1. Nodes within any bus, thus using unit addresses for children, shall be
>> + ordered incrementally by unit address.
>> + Alternatively for some sub-architectures, nodes of the same type can be
>> + grouped together (e.g. all I2C controllers one after another even if this
>> + breaks unit address ordering).
>> +
>> +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).
>> +
>> +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).
>
> Are these sub-architecture choices documented somewhere? Can you
> include a hint which they are?
This is a generic document, so it does not point to all possible
variations per each architecture or subarch. Just like Linux Coding
style does not cover all differences between subsystems.
>
>> +Example::
>> +
>> + /* SoC DTSI */
>> +
>> + / {
>
> Dumb question. Does this open { indicate the start of a bus?
>
>> + cpus {
>> + /* ... */
>> + };
>> +
>> + psci {
>> + /* ... */
>> + };
>
> If that does indicate a bus, the nodes above are ordered
> alpha-numerically, according to 2).
They are ordered. c is before p. p is before s.
>
>> +
>> + soc@ {
>
> This has a unit address, even if its missing, so should be sorted by
> 1).
And it is sorted...
>
> Should there be something in the coding style that 2) comes before 1)
> on the bus? And if that is true, don't you think it would make sense
> to swap 1) and 2) in the description above?
The root node is a bit special, but other than that mixing nodes with
and without unit address is discouraged practice.
>
>> + dma: dma-controller@10000 {
>> + /* ... */
>> + };
>> +
>> + clk: clock-controller@80000 {
>> + /* ... */
>> + };
>> + };
>> + };
>> +
>> + /* Board DTS - alphabetical order */
>> +
>> + &clk {
>> + /* ... */
>> + };
>> +
>> + &dma {
>> + /* ... */
>> + };
>> +
>> + /* Board DTS - alternative order, keep as DTSI */
>> +
>> + &dma {
>> + /* ... */
>> + };
>> +
>> + &clk {
>> + /* ... */
>> + };
>
> Do you imaging there will ever be a checkpatch for DT files? The
> second alternative seems pretty difficult to check for with tools. You
Rob pointed out that it is possible.
> need to include all the .dtsi files to determine the ordered tree,
> then flatten it to get the properties order. Should we discourage this
> alternative?
Please respond to Rob in v2 in such case.
>
>> +Indentation
>> +-----------
>> +
>> +1. Use indentation according to :ref:`codingstyle`.
>> +2. For arrays spanning across lines, it is preferred to align the continued
>> + entries with opening < from the first line.
>> +3. Each entry in arrays with multiple cells (e.g. "reg" with two IO addresses)
>> + shall be enclosed in <>.
>> +
>> +Example::
>> +
>> + thermal-sensor@c271000 {
>> + compatible = "qcom,sm8550-tsens", "qcom,tsens-v2";
>> + reg = <0x0 0x0c271000 0x0 0x1000>,
>> + <0x0 0x0c222000 0x0 0x1000>;
>> + };
>
> I'm not sure i understand this. Is this example correct?
>
> gpio-fan,speed-map = <0 0
> 3000 1
> 6000 2>;
>
> It exists a lot in todays files.
Depends on the binidng. Is it matrix? If yes, then it is not correct.
>
>
>> +The DTSI and DTS files shall be organized in a way representing the common
>> +(and re-usable) parts of the hardware. Typically this means organizing DTSI
>> +and DTS files into several files:
>> +
>> +1. DTSI with contents of the entire SoC (without nodes for hardware not present
>> + on the SoC).
>
> Maybe point out that SoC DTSI files can by hierarchical when there is
> a family of SoCs. You often have one .DTSI file for all the common
> parts of a family. And then each member of the family has a .dtsi file
> which includes the core, and then adds properties for that member of
> the family.
It's not really a coding style issue. We are going way to deep how
people should organize their source code. The only thing here I care is
to properly differentiate between SoC, SoM and board parts.
Best regards,
Krzysztof
