From e7294de8bf73688ca4520ac8a5a08b2a1a7930f8 Mon Sep 17 00:00:00 2001 From: James Roy Date: Thu, 12 Jun 2025 17:14:56 +0800 Subject: [PATCH 1/3] scripts: Add bindings style migration script This script replaces all underscore(_) separation in a binding with hyphens(-). Signed-off-by: James Roy --- scripts/utils/migrate_bindings_style.py | 81 +++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 scripts/utils/migrate_bindings_style.py diff --git a/scripts/utils/migrate_bindings_style.py b/scripts/utils/migrate_bindings_style.py new file mode 100644 index 000000000000..551ca05103d5 --- /dev/null +++ b/scripts/utils/migrate_bindings_style.py @@ -0,0 +1,81 @@ +#!/usr/bin/env python3 + +# Copyright (c) 2025 James Roy +# SPDX-License-Identifier: Apache-2.0 + +# This script is used to replace the properties containing +# underscores in the binding. + + +import argparse +import os +import sys +from pathlib import Path + +import yaml + +# bindings base +BINDINGS_PATH = [Path("dts/bindings/")] +BINDINGS_PROPERTIES_AL = None +with open(Path(__file__).parent / 'bindings_properties_allowlist.yaml') as f: + allowlist = yaml.safe_load(f.read()) + if allowlist is not None: + BINDINGS_PROPERTIES_AL = set(allowlist) + else: + BINDINGS_PROPERTIES_AL = set() + + +def parse_cmd_args(): + parse = argparse.ArgumentParser(allow_abbrev=False) + parse.add_argument("--path", nargs="+", help="User binding path directory", type=Path) + return parse.parse_args() + + +def get_yaml_binding_paths() -> list: + """Returns a list of 'dts/bindings/**/*.yaml'""" + from glob import glob + + bindings = [] + for path in BINDINGS_PATH: + yamls = glob(f"{os.fspath(path)}/**/*.yaml", recursive=True) + bindings.extend(yamls) + return bindings + + +def replace_bindings_underscores(binding_paths): + """ + Replace all property names containing underscores in the bindings. + """ + for binding_path in binding_paths: + with open(binding_path, encoding="utf-8") as f: + yaml_binding = yaml.safe_load(f) + properties = yaml_binding.get("properties", {}) + for prop_name in properties: + if '_' in prop_name and prop_name not in BINDINGS_PROPERTIES_AL: + _replace_underscores(binding_path, prop_name) + + +def _replace_underscores(binding_path, prop_name): + """Replace implementation""" + with open(binding_path, "r+", encoding="utf-8") as f: + lines = f.readlines() + for i, rowline in enumerate(lines): + line = rowline.split(":")[0].strip() + if prop_name == line and "#" not in rowline: + new_key = prop_name.replace("_", "-") + if new_key != line: + lines[i] = rowline.replace(line, new_key) + f.seek(0) + f.writelines(lines) + + +if __name__ == '__main__': + args = parse_cmd_args() + + if args.path is not None: + BINDINGS_PATH.extend(args.path) + + bindings = get_yaml_binding_paths() + replace_bindings_underscores(bindings) + + sys.exit(0) From ea55e12ead4540a0cc379fc2858aee8ccde0ffe9 Mon Sep 17 00:00:00 2001 From: James Roy Date: Thu, 12 Jun 2025 20:31:47 +0800 Subject: [PATCH 2/3] scripts: ci: Add CI bindings style checker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement a check in the CI pipeline to enforce that property names in device tree bindings do not contain underscores. Signed-off-by: James Roy Signed-off-by: Martí Bolívar --- scripts/bindings_properties_allowlist.yaml | 11 ++ scripts/ci/check_compliance.py | 117 +++++++++++++++------ 2 files changed, 98 insertions(+), 30 deletions(-) create mode 100644 scripts/bindings_properties_allowlist.yaml diff --git a/scripts/bindings_properties_allowlist.yaml b/scripts/bindings_properties_allowlist.yaml new file mode 100644 index 000000000000..100d4d380fbf --- /dev/null +++ b/scripts/bindings_properties_allowlist.yaml @@ -0,0 +1,11 @@ +# This file is a YAML allowlist of property names that are allowed to +# bypass the underscore check in bindings. These properties are exempt +# from the rule that requires using '-' instead of '_'. +# +# The file content can be as shown below: +# - propname1 +# - propname2 +# - ... + +- mmc-hs200-1_8v +- mmc-hs400-1_8v diff --git a/scripts/ci/check_compliance.py b/scripts/ci/check_compliance.py index 3cd5074c8f5b..91b49eb07877 100755 --- a/scripts/ci/check_compliance.py +++ b/scripts/ci/check_compliance.py @@ -21,6 +21,7 @@ import shutil import textwrap import unidiff +import yaml from yamllint import config, linter @@ -35,6 +36,30 @@ import list_boards import list_hardware +sys.path.insert(0, str(Path(__file__).resolve().parents[2] + / "scripts" / "dts" / "python-devicetree" / "src")) +from devicetree import edtlib + + +# Let the user run this script as ./scripts/ci/check_compliance.py without +# making them set ZEPHYR_BASE. +ZEPHYR_BASE = os.environ.get('ZEPHYR_BASE') +if ZEPHYR_BASE: + ZEPHYR_BASE = Path(ZEPHYR_BASE) +else: + ZEPHYR_BASE = Path(__file__).resolve().parents[2] + # Propagate this decision to child processes. + os.environ['ZEPHYR_BASE'] = str(ZEPHYR_BASE) + +# Initialize the property names allowlist +BINDINGS_PROPERTIES_AL = None +with open(Path(__file__).parents[1] / 'bindings_properties_allowlist.yaml') as f: + allowlist = yaml.safe_load(f.read()) + if allowlist is not None: + BINDINGS_PROPERTIES_AL = set(allowlist) + else: + BINDINGS_PROPERTIES_AL = set() + logger = None def git(*args, cwd=None, ignore_non_zero=False): @@ -380,32 +405,75 @@ class DevicetreeBindingsCheck(ComplianceTest): doc = "See https://docs.zephyrproject.org/latest/build/dts/bindings.html for more details." def run(self, full=True): - dts_bindings = self.parse_dt_bindings() - - for dts_binding in dts_bindings: - self.required_false_check(dts_binding) + bindings_diff, bindings = self.get_yaml_bindings() - def parse_dt_bindings(self): + # If no bindings are changed, skip this check. + try: + subprocess.check_call(['git', 'diff', '--quiet', COMMIT_RANGE] + + bindings_diff) + nodiff = True + except subprocess.CalledProcessError: + nodiff = False + if nodiff: + self.skip('no changes to bindings were made') + + for binding in bindings: + self.check(binding, self.check_yaml_property_name) + self.check(binding, self.required_false_check) + + @staticmethod + def check(binding, callback): + while binding is not None: + callback(binding) + binding = binding.child_binding + + def get_yaml_bindings(self): """ - Returns a list of dts/bindings/**/*.yaml files + Returns a list of 'dts/bindings/**/*.yaml' """ + from glob import glob + BINDINGS_PATH = 'dts/bindings/' + bindings_diff_dir, bindings = set(), [] - dt_bindings = [] - for file_name in get_files(filter="d"): - if 'dts/bindings/' in file_name and file_name.endswith('.yaml'): - dt_bindings.append(file_name) + for file_name in get_files(filter='d'): + if BINDINGS_PATH in file_name: + p = file_name.partition(BINDINGS_PATH) + bindings_diff_dir.add(os.path.join(p[0], p[1])) - return dt_bindings + for path in bindings_diff_dir: + yamls = glob(f'{os.fspath(path)}/**/*.yaml', recursive=True) + bindings.extend(yamls) - def required_false_check(self, dts_binding): - with open(dts_binding) as file: - for line_number, line in enumerate(file, 1): - if 'required: false' in line: - self.fmtd_failure( - 'warning', 'Devicetree Bindings', dts_binding, - line_number, col=None, - desc="'required: false' is redundant, please remove") + bindings = edtlib.bindings_from_paths(bindings, ignore_errors=True) + return list(bindings_diff_dir), bindings + def check_yaml_property_name(self, binding): + """ + Checks if the property names in the binding file contain underscores. + """ + for prop_name in binding.prop2specs: + if '_' in prop_name and prop_name not in BINDINGS_PROPERTIES_AL: + better_prop = prop_name.replace('_', '-') + print(f"Required: In '{binding.path}', " + f"the property '{prop_name}' " + f"should be renamed to '{better_prop}'.") + self.failure( + f"{binding.path}: property '{prop_name}' contains underscores.\n" + f"\tUse '{better_prop}' instead unless this property name is from Linux.\n" + "Or another authoritative upstream source of bindings for " + f"compatible '{binding.compatible}'.\n" + "\tHint: update 'bindings_properties_allowlist.yaml' if you need to " + "override this check for this property." + ) + + def required_false_check(self, binding): + raw_props = binding.raw.get('properties', {}) + for prop_name, raw_prop in raw_props.items(): + if raw_prop.get('required') is False: + self.failure( + f'{binding.path}: property "{prop_name}": ' + "'required: false' is redundant, please remove" + ) class KconfigCheck(ComplianceTest): """ @@ -1996,17 +2064,6 @@ def _main(args): # The "real" main(), which is wrapped to catch exceptions and report them # to GitHub. Returns the number of test failures. - global ZEPHYR_BASE - ZEPHYR_BASE = os.environ.get('ZEPHYR_BASE') - if not ZEPHYR_BASE: - # Let the user run this script as ./scripts/ci/check_compliance.py without - # making them set ZEPHYR_BASE. - ZEPHYR_BASE = str(Path(__file__).resolve().parents[2]) - - # Propagate this decision to child processes. - os.environ['ZEPHYR_BASE'] = ZEPHYR_BASE - ZEPHYR_BASE = Path(ZEPHYR_BASE) - # The absolute path of the top-level git directory. Initialize it here so # that issues running Git can be reported to GitHub. global GIT_TOP From 5daa5d79c1f91aa5c463638fd891d4018d295d40 Mon Sep 17 00:00:00 2001 From: James Roy Date: Thu, 12 Jun 2025 20:31:57 +0800 Subject: [PATCH 3/3] doc: Add a new migration guide entry Bindings now use hyphens(-) instead of underscores(_) as property separators. Signed-off-by: James Roy --- doc/releases/migration-guide-4.2.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/releases/migration-guide-4.2.rst b/doc/releases/migration-guide-4.2.rst index e382db763e5d..1ffe7b5196f2 100644 --- a/doc/releases/migration-guide-4.2.rst +++ b/doc/releases/migration-guide-4.2.rst @@ -100,6 +100,11 @@ Devicetree * The :c:macro:`DT_ENUM_HAS_VALUE` and :c:macro:`DT_INST_ENUM_HAS_VALUE` macros are now checking all values, when used on an array, not just the first one. +* Property names in devicetree and bindings use hyphens(``-``) as separators, and replacing + all previously used underscores(``_``). For local code, you can migrate property names in + bindings to use hyphens by running the ``scripts/migrate_bindings_style.py`` script. + + DAI ===