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

edtlib: add hash attribute to nodes #83748

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions include/zephyr/devicetree.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,16 @@
*/
#define DT_HAS_ALIAS(alias_name) DT_NODE_EXISTS(DT_ALIAS(alias_name))

/**
* @brief Get the hash associated with a DT node
*
* Get the hash for the specified node_id. The hash is calculated on the
* full devicetree path of the node.
* @param node_id node identifier
* @return hash value as a preprocessor token
*/
#define DT_HASH(node_id) DT_CAT(node_id, _HASH)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#define DT_HASH(node_id) DT_CAT(node_id, _HASH)
#define DT_NODE_HASH(node_id) DT_CAT(node_id, _HASH)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this is closely related to the node hierarchy, so HASH should not be used as a secondary naming convention. So, we should clearly label it as the function for obtaining the node's hash.


/**
* @brief Get a node identifier for an instance of a compatible
*
Expand Down
3 changes: 3 additions & 0 deletions scripts/dts/gen_defines.py
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,9 @@ def fmt_dep_list(dep_list):
else:
return "/* nothing */"

out_comment("Node's hash:")
out_dt_define(f"{node.z_path_id}_HASH", node.hash)

out_comment("Node's dependency ordinal:")
out_dt_define(f"{node.z_path_id}_ORD", node.dep_ordinal)
out_dt_define(f"{node.z_path_id}_ORD_STR_SORTABLE", f"{node.dep_ordinal:0>5}")
Expand Down
24 changes: 24 additions & 0 deletions scripts/dts/python-devicetree/src/devicetree/edtlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@
from dataclasses import dataclass
from typing import (Any, Callable, Iterable, NoReturn,
Optional, TYPE_CHECKING, Union)
import base64
import hashlib
import logging
import os
import re
Expand All @@ -90,6 +92,14 @@
from devicetree.grutils import Graph
from devicetree._private import _slice_helper

def _compute_hash(path: str) -> str:
# Calculates the hash associated with the node's full path.
hasher = hashlib.sha256()
hasher.update(path.encode())
hash = base64.b64encode(hasher.digest()).decode()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why encode it with base64? Maybe use hexdigest instead?

Copy link
Collaborator Author

@pillo79 pillo79 Jan 10, 2025

Choose a reason for hiding this comment

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

My first version was with hexdigest(), but that is a 64 char string and thus the final identifiers ended up well over 80. Using base64 as was suggested in this thread trims ~20 chars off and makes them easier to distinguish, at basically the same vanishingly small collision probability.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Understood, b64encode can be passed alternatives for the + and / chars already:

return base64.b64encode(hasher.digest(), altchars="__").decode().rstrip("=")

# replace invalid chars and drop padding
return hash.translate(hash.maketrans("+/", "__", "="))

#
# Public classes
#
Expand Down Expand Up @@ -912,6 +922,11 @@ class Node:
The ordinal is defined for all Nodes, and is unique among nodes in its
EDT 'nodes' list.
hash:
A hashed value of the devicetree path of the node. This is defined for
all Nodes, and is checked for uniqueness among nodes in its EDT 'nodes'
list.
required_by:
A list with the nodes that directly depend on the node
Expand Down Expand Up @@ -1027,6 +1042,7 @@ def __init__(
self.interrupts: list[ControllerAndData] = []
self.pinctrls: list[PinCtrl] = []
self.bus_node = self._bus_node(support_fixed_partitions_on_any_bus)
self.hash: str = _compute_hash(dt_node.path)

self._init_binding()
self._init_regs()
Expand Down Expand Up @@ -2270,10 +2286,18 @@ def _init_nodes(self) -> None:
# Creates a list of edtlib.Node objects from the dtlib.Node objects, in
# self.nodes

hash2node: dict[str, Node] = {}

for dt_node in self._dt.node_iter():
# Warning: We depend on parent Nodes being created before their
# children. This is guaranteed by node_iter().
node = Node(dt_node, self, self._fixed_partitions_no_bus)

if node.hash in hash2node:
_err(f"hash collision between '{node.path}' and "
f"'{hash2node[node.hash].path}'")
hash2node[node.hash] = node

self.nodes.append(node)
self._node2enode[dt_node] = node

Expand Down
10 changes: 10 additions & 0 deletions scripts/dts/python-devicetree/tests/test_edtlib.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add a test for macro DT_HASH(or DT_NODE_HASH) in tests/lib/devicetree/api/src/main.c? I think this would be better, as the ultimate audience is C, not edtlib.

Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,16 @@ def test_hierarchy():

assert edt.get_node("/parent/child-1").children == {}

def test_hashes():
'''Test Node.hash on hierarchy nodes'''
with from_here():
edt = edtlib.EDT("test.dts", ["test-bindings"])

assert edt.get_node("/").hash == "il7asoJjJEMhngUeSt4tHVu8Zxx4EFG_FDeJfL3_oPE"
assert edt.get_node("/parent").hash == "38lte65vn2I4YtFxqM921ZudsWz2sWGcCxAYf5EsKEQ"
assert edt.get_node("/parent/child-1").hash == "sv1LnIlXN4B1lRFuZL4WHWu9yfzRUhmY_kwWqkKr1Og"
assert edt.get_node("/parent/child-2").hash == "ayvZczwhPy5g_cnGHKwwWICJ7uD0gMOHAU06nNILOsI"

def test_child_index():
'''Test Node.child_index.'''
with from_here():
Expand Down
Loading