linux-kernel - Re: [PATCH v2] docs: dt-bindings: add DTS Coding Style document

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <01f9ce3b-e6e5-4b05-bf7f-0b3a5f74910a@linaro.org>
Date:   Wed, 22 Nov 2023 09:05:04 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Rafał Miłecki <zajec5@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org
Cc:     Andrew Davis <afd@...com>, Arnd Bergmann <arnd@...db.de>,
        Bjorn Andersson <andersson@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Heiko Stuebner <heiko@...ech.de>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Michal Simek <michal.simek@....com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Nishanth Menon <nm@...com>, Olof Johansson <olof@...om.net>,
        linux-rockchip@...ts.infradead.org,
        linux-samsung-soc@...r.kernel.org,
        linux-amlogic@...ts.infradead.org, linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v2] docs: dt-bindings: add DTS Coding Style document

On 21/11/2023 14:50, Rafał Miłecki wrote:
>> +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 should 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 should be
>> +   ordered alpha-numerically.
> 
> Just an idea. Would that make (more) sense to make &label-like entries
> match order of nodes in included .dts(i)?
> 
> Adventages:
> 1. We keep unit address incremental order that is unlikely to change
> 
> Disadventages:
> 1. More difficult to verify

Rob also proposed this and I believe above disadvantage here is crucial.
If you add new SoC with board DTS you are fine. But if you add only new
board, the order of entries look random in the diff hunk. Reviewer must
open SoC DTSI to be able to review the patch with board DTS.

If review is tricky and we do not have tool to perform it automatically,
I am sure submissions will have disordered board DTS.

> 
> 
>> +Example::
>> +
>> +	// SoC DTSI
>> +
>> +	/ {
>> +		cpus {
>> +			// ...
>> +		};
>> +
>> +		psci {
>> +			// ...
>> +		};
>> +
>> +		soc@ {
>> +			dma: dma-controller@...00 {
>> +				// ...
>> +			};
>> +
>> +			clk: clock-controller@...00 {
>> +				// ...
>> +			};
>> +		};
>> +	};
>> +
>> +	// Board DTS
>> +
>> +	&clk {
>> +		// ...
>> +	};
>> +
>> +	&dma {
>> +		// ...
>> +	};
>> +
>> +
>> +Order of Properties in Device Node
>> +----------------------------------
>> +
>> +Following order of properties in device nodes is preferred:
>> +
>> +1. compatible
>> +2. reg
>> +3. ranges
>> +4. Standard/common properties (defined by common bindings, e.g. without
>> +   vendor-prefixes)
>> +5. Vendor-specific properties
>> +6. status (if applicable)
>> +7. Child nodes, where each node is preceded with a blank line
>> +
>> +The "status" property is by default "okay", thus it can be omitted.
> 
> I think it would really help to include position of #address-cells and
> #size-cells here. In some files I saw them above "compatible" that seems
> unintuitive. Some prefer putting them at end which I think makes sense
> as they affect children nodes.
> 
> Whatever you choose it'd be just nice to have things consistent.

This is a standard/common property, thus it goes to (4) above.


Best regards,
Krzysztof