ST DTS lint #90611

yishai1999 · 2025-05-26T14:35:52Z

Fixed two issues according to the device tree guidelines https://docs.kernel.org/devicetree/bindings/dts-coding-style.html:

The reg property should be second only to the compatible property within a node. This was not the case in many partition nodes definitions.
The status property should be the last property in a node.

etienne-lms

Indeed these changes are recommended.
A few review comments.

etienne-lms · 2025-05-26T14:49:55Z

boards/st/b_l072z_lrwan1/b_l072z_lrwan1.dts

@@ -117,6 +117,7 @@ arduino_i2c: &i2c1 {};
 		     &spi1_miso_pa6 &spi1_mosi_pa7>;
 	pinctrl-names = "default";
 	status = "okay";
+


from the specs, I guess we should rather have:

pinctrl-names = "default"; + cs-gpios = <&gpioa 15 GPIO_ACTIVE_LOW>; status = "okay"; - cs-gpios = <&gpioa 15 GPIO_ACTIVE_LOW>; lora: lora@0 {

This exists in a bunch of places. I think it falls into the category of splitting into paragraphs for readabillity. It's nice to have the cs-gpios right before the spi device sub-nodes because they go together logically

I must say I don't understand why you don't want to change this while still change other status properties placement.
(2 other occurrences).

Did I fix all the inconsistancies?

etienne-lms · 2025-05-26T14:52:48Z

boards/st/sensortile_box/sensortile_box.dts

@@ -170,6 +170,7 @@
 	pinctrl-0 = <&spi2_sck_pd1 &spi2_miso_pd3 &spi2_mosi_pc3>;
 	pinctrl-names = "default";
 	status = "okay";
+


pinctrl-names = "default"; + cs-gpios = <&gpiod 0 GPIO_ACTIVE_LOW>; status = "okay"; - cs-gpios = <&gpiod 0 GPIO_ACTIVE_LOW>; spbtle_1s_sensortile_box: spbtle-1s@0 {

I still believe, if following the recommendation, that cs-gpios = ... should come before status = ....

boards/st/b_l4s5i_iot01a/b_l4s5i_iot01a.dts

boards/st/nucleo_f446ze/nucleo_f446ze.dts

boards/st/stm32n6570_dk/stm32n6570_dk_common.dtsi

yishai1999 · 2025-06-04T12:29:01Z

@etienne-lms All good?

etienne-lms

Aside the 3 comments below, the changes here are consistent.
(edited) the 3 comments are here, here and here.

@erwango, what do you think about such changes?

The DTS spec talks about a "preferred" order that is addressed in these changes (in https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#order-of-properties-in-device-node). It's not a strong recommendation.

The following order of properties in device nodes is preferred:

“compatible”

“reg”

“ranges”

Standard/common properties (defined by common bindings, e.g. without vendor-prefixes)

Vendor-specific properties

“status” (if applicable)

Child nodes, where each node is preceded with a blank line

There are a few changes in this P-R that I'm fully in favor of: rules 7. (sub nodes always after properties). As for the other, I'm not sure. That said, I must admit that always expecting the status property at this end is simpler, to know whether it's present or not, especially in case there are a lot of properties.

boards/st/stm32f429i_disc1/stm32f429i_disc1.dts

etienne-lms · 2025-06-06T11:49:50Z

boards/st/b_l072z_lrwan1/b_l072z_lrwan1.dts

@@ -117,6 +117,7 @@ arduino_i2c: &i2c1 {};
 		     &spi1_miso_pa6 &spi1_mosi_pa7>;
 	pinctrl-names = "default";
 	status = "okay";
+


I must say I don't understand why you don't want to change this while still change other status properties placement.
(2 other occurrences).

etienne-lms · 2025-06-06T12:07:45Z

boards/st/stm32l4r9i_disco/stm32l4r9i_disco.dts


 	mx25lm51245: ospi-nor-flash@90000000 {
-		status = "okay";


Equivalent change in boards/st/stm32l562e_dk/stm32l562e_dk_common.dtsi is missing.

Missed it. Will fix

According to the guidelines the `reg` property should always come before any other property other than `compatible`. Link: https://docs.kernel.org/devicetree/bindings/dts-coding-style.html Signed-off-by: Yishai Jaffe <[email protected]>

According to the guidelines the `status` property should always be the last property listed. Link: https://docs.kernel.org/devicetree/bindings/dts-coding-style.html Signed-off-by: Yishai Jaffe <[email protected]>

sonarqubecloud · 2025-06-09T08:18:20Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

erwango

Thanks for the effort and care provided to these files.

This being said, I'm a bit concerned by the spread of the change and:

the probability it will create churn to anyone that has PR/branch/patch on those files
the fact that this change is only a one time thing and will be hard to maintain (one going and next PR should be reviewed with the same requirements) and not enforced by linting script/checker, so won't be likely enforced in future
applied only on part of the files (st boards), so most probably, contamination from other areas will happen some day.

For all these reasons, I'd suggest to drop 2 parts to minimize the spread of the change:

status moves
label moves

I hope you understand my concern.

github-actions · 2025-08-19T00:39:07Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

etienne-lms

I'm still not sure the recommendations on status and label properties ordering in nodes really need to be strictly followed. That said, these changes almost look good to me, see my few remaining review comments.

etienne-lms · 2025-08-19T10:40:43Z

boards/st/nucleo_f767zi/nucleo_f767zi.dts

 			reg = <0x0 DT_SIZE_K(64)>;
+			label = "mcuboot";


This change belongs to previous commit "boards: st: change order of reg property in partitions."
Ditto for changes around lines 241, 253, 261 and 269.

etienne-lms · 2025-08-19T10:43:47Z

boards/st/sensortile_box/sensortile_box.dts

@@ -170,6 +170,7 @@
 	pinctrl-0 = <&spi2_sck_pd1 &spi2_miso_pd3 &spi2_mosi_pc3>;
 	pinctrl-names = "default";
 	status = "okay";
+


I still believe, if following the recommendation, that cs-gpios = ... should come before status = ....

etienne-lms · 2025-08-19T10:44:01Z

boards/st/sensortile_box_pro/sensortile_box_pro.dts

 	status = "okay";

+	cs-gpios = <&gpioa 2 GPIO_ACTIVE_LOW>;


I still believe, if following the recommendation, that cs-gpios = ... should come before status = ....

etienne-lms · 2025-08-19T10:47:35Z

boards/st/stm32l562e_dk/stm32l562e_dk_common.dtsi

@@ -234,7 +234,6 @@ stm32_lp_tick_source: &lptim1 {
 		spi-bus-width = <OSPI_OPI_MODE>;
 		data-rate = <OSPI_DTR_TRANSFER>;
 		four-byte-opcodes;
-		status = "okay";


status = "okay" is missing below sfdp-bfp property.

github-actions bot added the platform: STM32 ST Micro STM32 label May 26, 2025

github-actions bot requested review from asm5878 and benothmn-st May 26, 2025 14:36

yishai1999 force-pushed the st-dts-lint branch from 18de262 to 48619e3 Compare May 26, 2025 14:36

github-actions bot requested review from djiatsaf-st, erwango, etienne-lms, FRASTM, gautierg-st, GeorgeCGV, HoZHel, marwaiehm-st and mathieuchopstm May 26, 2025 14:36

github-actions bot assigned erwango May 26, 2025

etienne-lms reviewed May 26, 2025

View reviewed changes

yishai1999 force-pushed the st-dts-lint branch from 48619e3 to 33b4453 Compare May 28, 2025 15:07

etienne-lms reviewed Jun 6, 2025

View reviewed changes

yishai1999 force-pushed the st-dts-lint branch from 33b4453 to 813a560 Compare June 8, 2025 08:41

yishai1999 force-pushed the st-dts-lint branch from 813a560 to 64b844a Compare June 8, 2025 08:45

yishai1999 requested a review from etienne-lms June 8, 2025 09:21

boards: st: moved status property to end the of nodes

a96ae4f

According to the guidelines the `status` property should always be the last property listed. Link: https://docs.kernel.org/devicetree/bindings/dts-coding-style.html Signed-off-by: Yishai Jaffe <[email protected]>

yishai1999 force-pushed the st-dts-lint branch from 64b844a to a96ae4f Compare June 9, 2025 08:14

erwango requested changes Jun 9, 2025

View reviewed changes

yishai1999 added area: Devicetree area: Coding Guidelines Coding guidelines and style labels Jun 19, 2025

github-actions bot added the Stale label Aug 19, 2025

etienne-lms reviewed Aug 19, 2025

View reviewed changes

ST DTS lint #90611

Are you sure you want to change the base?

ST DTS lint #90611

Conversation

yishai1999 commented May 26, 2025

Uh oh!

etienne-lms left a comment

Choose a reason for hiding this comment

Uh oh!

etienne-lms May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yishai1999 commented Jun 4, 2025

Uh oh!

etienne-lms left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jun 9, 2025

Quality Gate passed

Uh oh!

erwango left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 19, 2025

Uh oh!

etienne-lms left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

etienne-lms May 26, 2025 •

edited

Loading

etienne-lms left a comment •

edited

Loading