Skip to content

Conversation

@anangl
Copy link
Member

@anangl anangl commented Feb 27, 2019

This patch adds generation of *_PARENT_LABEL macros for nodes that have a parent with a 'label' property and also have a 'label' property themselves.
Such feature was requested by @pizi-nordic in order to create the device hierarchy needed in the power management rework he's doing.

Fixes: #13313

@galak galak added this to the future milestone Feb 27, 2019
@codecov-io
Copy link

codecov-io commented Feb 27, 2019

Codecov Report

Merging #13905 into master will decrease coverage by 0.85%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13905      +/-   ##
==========================================
- Coverage   52.83%   51.98%   -0.86%     
==========================================
  Files         310      309       -1     
  Lines       45311    45583     +272     
  Branches    10468    10555      +87     
==========================================
- Hits        23942    23695     -247     
- Misses      16584    17080     +496     
- Partials     4785     4808      +23
Impacted Files Coverage Δ
subsys/net/ip/net_pkt.c 44.92% <0%> (-24.44%) ⬇️
subsys/net/ip/udp.c 47.66% <0%> (-21.79%) ⬇️
include/spinlock.h 57.14% <0%> (-17.86%) ⬇️
kernel/mutex.c 83.82% <0%> (-13.28%) ⬇️
subsys/net/ip/ipv4.c 51.9% <0%> (-12.96%) ⬇️
subsys/net/ip/net_context.c 49.41% <0%> (-9.47%) ⬇️
kernel/timeout.c 75.92% <0%> (-9.26%) ⬇️
kernel/include/kernel_structs.h 90.9% <0%> (-9.1%) ⬇️
kernel/sched.c 84.01% <0%> (-8.74%) ⬇️
subsys/net/ip/ipv6.c 50.63% <0%> (-8.44%) ⬇️
... and 71 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f90acbf...57d39b9. Read the comment docs.

@anangl anangl force-pushed the wip_add_parent_label_macros_v2 branch from 5c6adb8 to 9cb3214 Compare February 27, 2019 16:54
@anangl
Copy link
Member Author

anangl commented Feb 27, 2019

Rephrased the commit description.

Copy link
Contributor

@ulfalizer ulfalizer Feb 27, 2019

Choose a reason for hiding this comment

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

get_parent_path('/') currently returns '/', so the parent_path check will always succeed.

Maybe the check could be node_path != '/' instead, if it's needed.

Or get_parent_path() could be changed to return None for /.

Copy link
Contributor

Choose a reason for hiding this comment

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

Returning None for / might be best. Just ran into another case where it's useful.

Copy link
Contributor

Choose a reason for hiding this comment

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

Did that in #13922. Just added

    if node_path == '/':
        return None

to the start of get_parent_path().

Copy link
Member Author

Choose a reason for hiding this comment

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

Did that in #13922.

Okay. So this PR just needs to wait until #13922 gets merged.
Actually, even with the current master everything will work. parent_path will always succeed, but for top-level nodes reduced['/'] won't have the 'label' property.

Copy link
Member Author

@anangl anangl Feb 28, 2019

Choose a reason for hiding this comment

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

I just took another look. Checking parent_path here doesn't make much sense, as it will never be None. This code is executed only for nodes that have the compatible property, hence for path '/' we won't get here. I think I'll remove this check.

Copy link
Contributor

@ulfalizer ulfalizer Feb 28, 2019

Choose a reason for hiding this comment

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

Yeah, no need to avoid merge conflicts with those cleanup PRs either.

@anangl anangl force-pushed the wip_add_parent_label_macros_v2 branch from 9cb3214 to ac1939f Compare February 28, 2019 09:23
@anangl
Copy link
Member Author

anangl commented Feb 28, 2019

Rebased on master.

@anangl anangl force-pushed the wip_add_parent_label_macros_v2 branch 2 times, most recently from 764b7fe to fda2b31 Compare February 28, 2019 09:25
@anangl
Copy link
Member Author

anangl commented Feb 28, 2019

Removed the no longer needed parent_path check.

Copy link
Contributor

@ulfalizer ulfalizer left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

This patch adds generation of `*_PARENT_LABEL` macros for nodes
that have a parent with a 'label' property and also have a 'label'
property themselves.

Signed-off-by: Andrzej Głąbek <[email protected]>
@anangl anangl force-pushed the wip_add_parent_label_macros_v2 branch from fda2b31 to 57d39b9 Compare March 19, 2019 07:38
@anangl
Copy link
Member Author

anangl commented Mar 19, 2019

  • rebased on master
  • renamed the parameter added to extract_parent_label: label_postfix -> def_label_postfix

@nashif
Copy link
Member

nashif commented Sep 19, 2019

@anangl are you going to update this PR? Is this still relevant?

@anangl
Copy link
Member Author

anangl commented Sep 19, 2019

@anangl are you going to update this PR? Is this still relevant?

No, I'll close it.

@anangl anangl closed this Sep 19, 2019
@anangl anangl deleted the wip_add_parent_label_macros_v2 branch March 11, 2020 08:00
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.

Generate hierarchy while parsing DTS

5 participants