Skip to content
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

cli: Add command "dts-check" to validate dts files and improve board & patch development overall (resubmission) #6739

Merged
merged 4 commits into from
Jun 25, 2024

Conversation

ColorfulRhino
Copy link
Collaborator

@ColorfulRhino ColorfulRhino commented Jun 13, 2024

Description

This is a resubmission of #6482 which was reverted in #6524 due to build errors. I don't know what exactly failed last time and my build tests this time run successfully just fine on Ubuntu 24.04. Maybe the error is specific to certain configurations and will return, so please don't merge until CI has done some tests and a build was successful.

I have slightly improved this latest version by moving the function call to validate_dts in kernel.sh slightly further down after kernel_config ran since make expects a .config file.

Original description

This new command can be used like this:

./compile.sh BOARD=nanopi-r5c BRANCH=edge dts-check

It validates the dts/dtb file for the selected board against dt bindings and outputs the validation logs to the user.
This can be used when adding a new board, developing or improving a dts file. Should lead to higher quality device trees and patches overall, if used.

Documentation summary for feature

(This was already added to the documentation in armbian/documentation@4cf2ff4)

  • short description (copy / paste of PR title)
    cli: Add command "dts-check" to validate dts files and improve board & patch development overall
  • summary (description relevant for end users)
    This command validates the dts/dtb file for the selected board against device tree bindings and outputs the validation logs to the user.
    It can be used when adding a new board, developing or improving a dts file. Should lead to higher quality device trees and patches overall, if used.
  • example of usage (how to see this in function)
./compile.sh BOARD=nanopi-r5c BRANCH=edge dts-check

Then wait for the validation to finish and check the output. If there is anything wrong with your board's device tree, you will see errors or warnings in the log. Analyze them carefully and use this information to improve your dts file.
Please note: Errors or warnings can also be introduced by SoC/board patches. When patching in new functions, like crypto for the RK3588, not only add the relevant parts to rk3588s.dtsi, but also try to add the device tree bindings in linux/Documentation/devicetree/bingings/ for validation.

How Has This Been Tested?

  • Command success without errors: ./compile.sh BOARD=nanopi-r5c BRANCH=edge dts-check
  • Normal build success without errors: ./compile.sh build BOARD=nanopi-r5c BRANCH=edge BUILD_DESKTOP=no BUILD_MINIMAL=no EXPERT=yes EXTRAWIFI=no KERNEL_CONFIGURE=no RELEASE=trixie
  • Check that Python Pip hashing still works with requirements.txt by changing the package versions
  • Testing with another configuration/machine is needed to avoid the mentioned build error

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

@ColorfulRhino ColorfulRhino requested a review from a team as a code owner June 13, 2024 22:41
@github-actions github-actions bot added the size/medium PR with more then 50 and less then 250 lines label Jun 13, 2024
@ColorfulRhino ColorfulRhino added the Build Executing build train (permission needed) label Jun 13, 2024
@github-actions github-actions bot added the Framework Framework components label Jun 13, 2024
@ColorfulRhino ColorfulRhino added Build Executing build train (permission needed) and removed Build Executing build train (permission needed) labels Jun 17, 2024
@igorpecovnik igorpecovnik added the Work in progress Unfinished / work in progress label Jun 24, 2024
@ColorfulRhino ColorfulRhino removed Work in progress Unfinished / work in progress labels Jun 24, 2024
Copy link
Member

@rpardini rpardini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, lets merge.

(Keep this in mind though as the PATH change for make might somehow impact some old/crazy/vendor kernels -- lets see)

Validates the dts/dtb file for the selected board and outputs the validation logs to the user.
This can be used when adding a new board, developing or improving a dts file. Should lead to higher quality device trees and patches overall, if used.
Will show warnings/errors if patches patch in some functionalities to a devicetree file without patching in the dt-bindings .yaml at the same time.
This makes dependencies easier to track and opens up the possibility for Dependabot to update them.
@ColorfulRhino
Copy link
Collaborator Author

ColorfulRhino commented Jun 24, 2024

(Keep this in mind though as the PATH change for make might somehow impact some old/crazy/vendor kernels -- lets see)

Which Path?
I mean, in my opinion, it's time to let go of all pre-6.1 kernels anyway. Makes the codebase cleaner :)

Please keep in mind though, last time these changes worked perfectly fine on my machine, but CI had some issues.
We should keep an eye out after merging this :)

@ColorfulRhino ColorfulRhino added Ready to merge Reviewed, tested and ready for merge 08 Milestone: Third quarter release labels Jun 24, 2024
@igorpecovnik igorpecovnik merged commit cade8cf into armbian:main Jun 25, 2024
8 of 11 checks passed
@ColorfulRhino ColorfulRhino deleted the dtb-check_new branch June 25, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
08 Milestone: Third quarter release Build Executing build train (permission needed) Framework Framework components Ready to merge Reviewed, tested and ready for merge size/medium PR with more then 50 and less then 250 lines
Development

Successfully merging this pull request may close these issues.

3 participants