-
Notifications
You must be signed in to change notification settings - Fork 7k
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
scripts: extract_dts_includes.py: refactor for better maintenance #8541
Conversation
Refactor for better maintenance and to ease future enhancements. Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #8541 +/- ##
==========================================
- Coverage 63.92% 63.24% -0.69%
==========================================
Files 427 441 +14
Lines 41100 45629 +4529
Branches 6939 8269 +1330
==========================================
+ Hits 26275 28860 +2585
- Misses 11678 13276 +1598
- Partials 3147 3493 +346
Continue to review full report at Codecov.
|
def __init__(self): | ||
pass | ||
|
||
def _extract_consumer(self, node_address, yaml, clocks, names, defs, def_label): |
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.
Can you remove this module in the PR?
This is new material that would deserve a dedicated review with matching yaml files and examples.
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.
This is the clocks directive (eg. BUS, BITS) as before. extract_cells and extract_controller
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.
Yeah, and this is actually a bit more than that as you added #clock-cells treatment.
I don't say this is not correct, but that this is more than the initial status of script features.
I've checked what was generated and it seems true to what was generated earlier.
What puzzles me is that there is more code than in initial function that was generic to other nodes..
# @param[out] defs Property definitions for each node address | ||
# @param def_label Define label string of node owning the directive. | ||
# | ||
def extract(self, node_address, yaml, prop, names, defs, def_label): |
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.
defs
is now global (defined in globals.py) , remove from parameters list.
Apply everywhere
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 know that defs is in globals. But the interface was kept. See current version of extract_dts_inlcudes.py. Do you really want this new interface in a refactor only PR?
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.
ok, my bad. This was done in my local branch bu not merged actually.
You can discard this comment
## | ||
# @brief Extract directives in a default way | ||
# | ||
# @param node_address Address of node owning the clockxxx definition. |
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 guess this comment is not accurate
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.
Yes should read clocks (clockxxx is for future use).
Approved. Let's get this merged |
Refactor for better maintenance and to ease future enhancements.
This is a cutout of PR #6761 (which is a cutout of #5799) that can be applied independently.
Signed-off-by: Bobby Noelte b0661n0e17e@gmail.com