-
Notifications
You must be signed in to change notification settings - Fork 8k
dts/board format property node order #96317
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
base: main
Are you sure you want to change the base?
dts/board format property node order #96317
Conversation
990a9d4
to
eb70cba
Compare
Is this enforced in the current version of linter? |
No not yet PR in the dts-lsp is still pending to be merged depending on the outcome of this PR i.e. #92334 can go in main, then we can bump up the version of the Linter should we agree on this PR |
eb70cba
to
40b2a98
Compare
Applying dts-linter results for format files in boards/01space Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/96boards Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/aconno Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/acrn Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/actinius Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/adafruit Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/adi Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/aesc Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/aithinker Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/alientek Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/ambiq Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/amd Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/andestech Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/antmicro Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/arduino Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/arm Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/aspeed Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/atmarktechno Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/atmel Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/bbc Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/bcdevices Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/beagle Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/bflb Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/tdk Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/technexion Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/telink Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/ti Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/toradex Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/u-blox Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/udoo Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/up-bridge-the-gap Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/variscite Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/vcc-gnd Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/vngiotlab Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/waveshare Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/wch Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/we Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/weact Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/wemos Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/witte Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/wiznet Signed-off-by: Kyle Micallef Bonnici <[email protected]>
Applying dts-linter results for format files in boards/xen Signed-off-by: Kyle Micallef Bonnici <[email protected]>
40b2a98
to
d73a807
Compare
|
|
||
cs-gpios = <&gpioc 0 GPIO_ACTIVE_LOW>; | ||
status = "okay"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in cases like this status
should either be in its own block or the empty line on line 192 should be removed. Either way, status
and cs-gpios
don't have a connection which would warrant them being in a block of their own. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I see it, currently we have "4 blocks/groups"
Block 1:
- “compatible”
- “reg”
- “ranges”
Block 2:
- Standard/common properties (defined by common bindings, e.g. without vendor-prefixes)
Block 3:
- Vendor-specific properties
Block 4:
- “status” (if applicable)
if we are to do something about status
perhaps should do some thing about the other blocks.
I am open to this if this if the coding standard we want to apply.
@nordicjm, @fabiobaltieri, @kartben , @keith-zephyr what are your thoughts
Not against any of these changes, but I do think we should have enforcement in CI before doing these cleanups. EDIT: If we want to enforce these preferred guidelines. |
So a challenge with this is that the following guideline is a MAY, not a SHALL: "The following order of properties in device nodes is preferred" ; are you planning on making it a warning in CI checks? This will likely be very verbose for folks though... |
This was a proof of concept to show what else we can enforce!. Side note, my idea here is to also open the door for us discuss if we want to do more. I am open for more ideas on what can help readability even if not this one. Back to this PR ... This could be mandatory, an additional flag in the linter/lsp the user may opt in or out of for there code base or non of the above. I am also not applying the last line of the style guide I do not think showing warning will be reasonable, as showing a warning is pointless if we do not enforce due to the quantity. Also, a warning hints something Is wrong and if will threat this as a preferred and not SHELL then nothing is wrong. The issue I personally had with the SHELL vs MAY is that in part of the style guide says 'shall' as while in other case it states preferred... such as I will leave it up to you all to decide what is best for the community and what you decide to enforce I will do my best make sure this tooling will respect it inside the linter and lsp. |
This PR addresse the style guide issues to ensure properties are on top on nodes and that properties are sorted as per
(https://docs.kernel.org/devicetree/bindings/dts-coding-style.html)
This PR does not address sorting of nodes.
The style guide says
If sorting of nodes is also desired, to avoid parsing bindings for linting to determine the bus type, "Nodes on any bus, thus using unit addresses for children, shall be ordered by unit address in ascending order." would be the only feasible solution IMO