On Wed, Nov 22, 2023 at 9:21 AM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > On 22/11/2023 09:09, Chen-Yu Tsai wrote: > > On Wed, Nov 22, 2023 at 4:05 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@xxxxxxxxxx> wrote: > >> 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@10000 { > >>>> + // ... > >>>> + }; > >>>> + > >>>> + clk: clock-controller@80000 { > >>>> + // ... > >>>> + }; > >>>> + }; > >>>> + }; > >>>> + > >>>> + // 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. > > > > It's probably a mix, but AFAIK a lot of the device trees in tree have > > #*-cells after "status". In some cases they are added in the board > > .dts files, not the chip/module .dtsi files. > > Existing DTS is not a good example :) > > > > > +1 that it makes sense at the end as they affect child nodes. > > I still insist that status must be the last, because: > 1. Many SoC nodes have address/size cells but do not have any children > (I2C, SPI), so we put useless information at the end. > 2. Status should be the final information to say whether the node is > ready or is not. I read the node, check properties and then look at the end: > a. Lack of status means it is ready. > b. status=disabled means device still needs board resources/customization +1 for status at the end (before children), so it's easy to find. Also, reality can look like the example in the bindings, with an optional status property appended. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds