Skip to content

dts: cosmetic improvements to output file layout #90804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

pillo79
Copy link
Collaborator

@pillo79 pillo79 commented May 29, 2025

After the introduction of source references in comments by #89410, this PR improves the layout of the generated zephyr.dts with the following adjustments:

  • Wrap long item arrays so that they don't create excessively long lines, messing comment formatting.

    There is now a lax limit of 80 chars for the array entries, so that the whole block can be read on most screens with a minimum of scrolling, and comments are readily visible after it.

    Example (click to open)

    Current: extra long lines and unreachable comment blocks

    /* node '/soc/memory-controller@52004000' defined in zephyr/dts/arm/st/h7/stm32h7.dtsi:1051 */
    fmc: memory-controller@52004000 {
            compatible = "st,stm32h7-fmc";                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                          /* in zephyr/dts/arm/st/h7/stm32h7.dtsi:1052 */
            reg = < 0x52004000 0x400 >;                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                             /* in zephyr/dts/arm/st/h7/stm32h7.dtsi:1053 */
            clocks = < &rcc 0xd4 0x1000 >;                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                          /* in zephyr/dts/arm/st/h7/stm32h7.dtsi:1054 */
            status = "okay";                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                        /* in zephyr/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts:211 */
            pinctrl-0 = < &fmc_nbl0_pe0 &fmc_nbl1_pe1 &fmc_sdclk_pg8 &fmc_sdnwe_ph5 &fmc_sdcke0_ph2 &fmc_sdne0_ph3 &fmc_sdnras_pf11 &fmc_sdncas_pg15 &fmc_a0_pf0 &fmc_a1_pf1 &fmc_a2_pf2 &fmc_a3_pf3 &fmc_a4_pf4 &fmc_a5_pf5 &fmc_a6_pf12 &fmc_a7_pf13 &fmc_a8_pf14 &fmc_a9_pf15 &fmc_a10_pg0 &fmc_a11_pg1 &fmc_a12_pg2 &fmc_a14_pg4 &fmc_a15_pg5 &fmc_d0_pd14 &fmc_d1_pd15 &fmc_d2_pd0 &fmc_d3_pd1 &fmc_d4_pe7 &fmc_d5_pe8 &fmc_d6_pe9 &fmc_d7_pe10 &fmc_d8_pe11 &fmc_d9_pe12 &fmc_d10_pe13 &fmc_d11_pe14 &fmc_d12_pe15 &fmc_d13_pd8 &fmc_d14_pd9 &fmc_d15_pd10 >; /* in zephyr/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts:212 */
            pinctrl-names = "default";                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              /* in zephyr/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts:225 */
    };

    With PR: lines are shortened to a manageable width

    /* node '/soc/memory-controller@52004000' defined in zephyr/dts/arm/st/h7/stm32h7.dtsi:1051 */
    fmc: memory-controller@52004000 {
            compatible = "st,stm32h7-fmc";                                                                          /* in zephyr/dts/arm/st/h7/stm32h7.dtsi:1052 */
            reg = < 0x52004000 0x400 >;                                                                             /* in zephyr/dts/arm/st/h7/stm32h7.dtsi:1053 */
            clocks = < &rcc 0xd4 0x1000 >;                                                                          /* in zephyr/dts/arm/st/h7/stm32h7.dtsi:1054 */
            status = "okay";                                                                                        /* in zephyr/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts:211 */
            pinctrl-0 = < &fmc_nbl0_pe0 &fmc_nbl1_pe1 &fmc_sdclk_pg8 &fmc_sdnwe_ph5 &fmc_sdcke0_ph2 &fmc_sdne0_ph3
                          &fmc_sdnras_pf11 &fmc_sdncas_pg15 &fmc_a0_pf0 &fmc_a1_pf1 &fmc_a2_pf2 &fmc_a3_pf3
                          &fmc_a4_pf4 &fmc_a5_pf5 &fmc_a6_pf12 &fmc_a7_pf13 &fmc_a8_pf14 &fmc_a9_pf15 &fmc_a10_pg0
                          &fmc_a11_pg1 &fmc_a12_pg2 &fmc_a14_pg4 &fmc_a15_pg5 &fmc_d0_pd14 &fmc_d1_pd15 &fmc_d2_pd0
                          &fmc_d3_pd1 &fmc_d4_pe7 &fmc_d5_pe8 &fmc_d6_pe9 &fmc_d7_pe10 &fmc_d8_pe11 &fmc_d9_pe12
                          &fmc_d10_pe13 &fmc_d11_pe14 &fmc_d12_pe15 &fmc_d13_pd8 &fmc_d14_pd9 &fmc_d15_pd10 >;      /* in zephyr/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts:212 */
            pinctrl-names = "default";                                                                              /* in zephyr/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts:225 */
    };
  • Keep properties in input DTS order in each node.

    Currently, when a property is updated, its location is not changed and still reflects the order in the initial definition. Grouping the changes makes it easier to see at a glance the block of lines each input file touches.

    Example (click to open)

    Current: properties in the output file appear to come from multiple scattered changes:

    /* node '/soc/dac@40007400' defined in zephyr/dts/arm/st/h7/stm32h7.dtsi:921 */
    dac1: dac@40007400 {
            compatible = "st,stm32-dac";                   /* in zephyr/dts/arm/st/h7/stm32h7.dtsi:922 */
            reg = < 0x40007400 0x400 >;                    /* in zephyr/dts/arm/st/h7/stm32h7.dtsi:923 */
            clocks = < &rcc 0xe8 0x20000000 >;             /* in zephyr/dts/arm/st/h7/stm32h7.dtsi:924 */
            status = "okay";                               /* in zephyr/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts:205 */
            #io-channel-cells = < 0x1 >;                   /* in zephyr/dts/arm/st/h7/stm32h7.dtsi:926 */
            pinctrl-0 = < &dac1_out1_pa4 &dac1_out2_pa5 >; /* in zephyr/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts:206 */
            pinctrl-names = "default";                     /* in zephyr/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts:207 */
            phandle = < 0x5f >;                            /* in zephyr/samples/drivers/dac/boards/arduino_giga_r1_stm32h747xx_m7.overlay:3 */
    };

    With this PR, properties are sorted by input file:

    /* node '/soc/dac@40007400' defined in zephyr/dts/arm/st/h7/stm32h7.dtsi:921 */
    dac1: dac@40007400 {
            compatible = "st,stm32-dac";                   /* in zephyr/dts/arm/st/h7/stm32h7.dtsi:922 */
            reg = < 0x40007400 0x400 >;                    /* in zephyr/dts/arm/st/h7/stm32h7.dtsi:923 */
            clocks = < &rcc 0xe8 0x20000000 >;             /* in zephyr/dts/arm/st/h7/stm32h7.dtsi:924 */
            #io-channel-cells = < 0x1 >;                   /* in zephyr/dts/arm/st/h7/stm32h7.dtsi:926 */
            status = "okay";                               /* in zephyr/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts:205 */
            pinctrl-0 = < &dac1_out1_pa4 &dac1_out2_pa5 >; /* in zephyr/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts:206 */
            pinctrl-names = "default";                     /* in zephyr/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts:207 */
            phandle = < 0x5f >;                            /* in zephyr/samples/drivers/dac/boards/arduino_giga_r1_stm32h747xx_m7.overlay:3 */
    };

Review notes:

No functional change to the DTS is introduced.

The noqa: F821 linter suppression prevents flagging array_start and array_newline as undefined variables.
This is because these variables are initialized when an opening brace is output (the else: #MarkerType.UINT* clause), which is necessarily before any element in a byte or prop array. A sequence of markers not following this pattern would indicate a bug in the DTS parsing code.

I also experimented with sorting nodes in the same way, but decided against it. I felt the order of nodes is usually cleaner in CPU/SoC DTS than in the board/application files, so keeping the initial definition order results in a cleaner overall structure of the output DTS.

pillo79 added 2 commits May 29, 2025 16:20
Split long arrays into multiple lines to improve readability of the
output DTS file. A new line is started when the array data exceeds
80 characters.

Add a few test entries to verify the new behavior.

Note: the F821 linter suppression prevents flagging 'array_start' and
'array_newline' as undefined variables. This is because these variables
are initialized when an opening brace is output, which is necessarily
before any element in a byte or prop array. A sequence of markers not
following this pattern would indicate a bug in the DTS parsing code.

Signed-off-by: Luca Burelli <[email protected]>
Ensure that the order of properties in the output DTS file matches the
order in which they are defined in the DTS source files by moving props
to the end of the dictionary when they are accessed.

Signed-off-by: Luca Burelli <[email protected]>
@pillo79 pillo79 force-pushed the pr-dts-layout-improvements branch from 41ac6e9 to 49d61ab Compare May 29, 2025 14:32
Copy link

@pillo79 pillo79 marked this pull request as ready for review May 30, 2025 07:01
@github-actions github-actions bot requested review from decsny, galak and rruuaanng May 30, 2025 07:01
@pillo79 pillo79 requested review from kartben and fabiobaltieri May 30, 2025 07:36
@danieldegrasse danieldegrasse merged commit ed7f2e0 into zephyrproject-rtos:main Jun 5, 2025
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants