Skip to content

Conversation

@erwango
Copy link
Member

@erwango erwango commented Jan 14, 2021

Add dt_node_has_prop function to query the presence of 'prop'
for given node label.

Cf use in #30864

Signed-off-by: Erwan Gouriou [email protected]

Add dt_node_has_prop function to query the presence of 'prop'
for given node label.

Signed-off-by: Erwan Gouriou <[email protected]>
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic left a comment

Choose a reason for hiding this comment

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

Can you implement both this and dt_node_has_bool_prop using a single function instead of copy/pasting? Similar to how dt_node_reg_size_int and dt_node_reg_size_hex are both implemented using dt_node_reg.

@erwango
Copy link
Member Author

erwango commented Jan 15, 2021

Can you implement both this and dt_node_has_bool_prop using a single function instead of copy/pasting? Similar to how dt_node_reg_size_int and dt_node_reg_size_hex are both implemented using dt_node_reg.

@mbolivar dt_node_has_bool_prop takes a 'path', while this function takes a 'label'. Do you want me to change this as well ?

EDIT: The 2 dt_node_has_bool_prop only occurrences are there:

config COUNTER_RTC_WITH_PPI_WRAP
def_bool ($(dt_node_has_bool_prop,rtc-0,ppi-wrap) && COUNTER_RTC0) || \

But I can't find any matching of rtc-x paths (as these are supposed to be).

Do you think I can safely change dt_node_has_bool_prop to take paths and use rtcx instead of rtc-x ?
Maybe you have downstream use of rtc-x?

@erwango
Copy link
Member Author

erwango commented Jan 15, 2021

@mbolivar-nordic This patch has been merged as is as part of #30864 (I probably didn't advertised enough this late dependency)
Though, it is not too late to fix as per your request, if you could answer previous comment (I guess with the correct @Nickname will work better)

@mbolivar-nordic
Copy link
Contributor

dt_node_has_bool_prop takes a 'path', while this function takes a 'label'. Do you want me to change this as well ?

Heh, yes, I didn't notice that on my first review, but it's a good reason to do it in terms of existing code.

The name dt_node_has_prop is not a good one then, IMO. It should be dt_nodelabel_has_prop; notice other functions like dt_nodelabel_has_compat.

Can you please send a follow-up making dt_node_has_prop and if necessary dt_nodelabel_has_prop?

@github-actions
Copy link

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.

@github-actions github-actions bot added the Stale label Mar 22, 2021
@github-actions github-actions bot closed this Apr 6, 2021
@erwango erwango deleted the dev_kconfig_has_prop branch January 27, 2022 13:37
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.

4 participants