Re: [PATCH 8/9] dtc: dt-check-style: Introduce 'stricter' mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Missing trailing commas in `applies_to` arguments cause them to be evaluated as strings instead of tuples, breaking the `--list-rules` output formatting.
- [Low] Misleading documentation for `Rule.mode` claims dictionary support, which would crash the script.
- [Low] Copy-paste error in the header comment of the `dts-line-length.dts` test fixture.
--

--- Patch [8]: [PATCH 8/9] dtc: dt-check-style: Introduce 'stricter' mode ---
commit 2b6676a210ee00e188dac94bab5f81848b3977f1
Author: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

dtc: dt-check-style: Introduce 'stricter' mode
    
Two rules, which are enabled in 'strict' mode make little sense for DTS:

1. line-length, limiting length of line to 80 characters: DTS often has
   a bit longer lines, especially for interconnects or heavily nested
   opp-level.  Checkpatch already does not warn for exceeding 80
   characters.

2. unused-labels, discouraging unused labels in DTS or YAML (not DTSI or
   DTSO): while not harming this leads to many false positives, e.g.
   unused PMIC regulators in DTS.

Introduce another 'mode' of running beside existing relaxed and strict:
a 'stricter' one where these two rules are moved for DTS.  Intention is
to have in-tree DTS passing 'strict' mode.

> diff --git a/scripts/dtc/dt-check-style b/scripts/dtc/dt-check-style
> index 8c049c04b865c..7e1d80ceab780 100755
> --- a/scripts/dtc/dt-check-style
> +++ b/scripts/dtc/dt-check-style

[ ... ]

> @@ -332,7 +334,8 @@ class Rule:
>      def __init__(self, name, mode, description, check,
>                   applies_to=('yaml', 'dts', 'dtsi', 'dtso')):
>          self.name = name
> -        self.mode = mode               # 'relaxed' or 'strict'
> +        # 'relaxed', 'strict', 'stricter' ordictionary mapping file_type to mode
> +        self.mode = mode

[Severity: Low]
This isn't a bug, but is there a typo ("ordictionary") in this comment? 

Also, if a dictionary were actually passed for mode based on this comment, 
wouldn't it cause a crash in select_rules() with a TypeError? The rank 
dictionary expects r.mode to be a hashable type rather than a dict.

[ ... ]

> @@ -1042,20 +1045,27 @@ RULES = [
>           check_node_close_alone),
>      Rule('line-length', 'strict',
>           'lines must not exceed 80 columns',
> -         check_line_length),
> +         check_line_length, applies_to=('yaml')),

[Severity: Medium]
Will python evaluate applies_to=('yaml') as a string rather than a 
single-element tuple because it lacks a trailing comma?

It looks like this causes the --list-rules output to iterate over the 
characters of the string instead, resulting in [y,a,m,l] when applies_to 
is consumed by ','.join(r.applies_to) in main(). 

Could a trailing comma be added like ('yaml',) to ensure it is treated 
as a tuple? This same issue seems to apply to the other single-element tuples 
added below for applies_to=('yaml') and applies_to=('dts').

> +    Rule('line-length-dts', 'stricter',
> +         'lines must not exceed 80 columns',
> +         check_line_length, applies_to=('dts', 'dtsi', 'dtso')),
>      Rule('continuation-alignment', 'strict',
>           'multi-line property continuations align under "<" or "\\""',
>           check_continuation_alignment),
>      Rule('unused-labels', 'strict',
>           'every label must be &-referenced in the same example/file '
>           '(skipped for .dtsi/.dtso since labels there are exported)',
> -         check_unused_labels, applies_to=('yaml', 'dts')),
> +         check_unused_labels, applies_to=('yaml')),
> +    Rule('unused-labels-dts', 'stricter',
> +         'every label must be &-referenced in the same example/file '
> +         '(skipped for .dtsi/.dtso since labels there are exported)',
> +         check_unused_labels, applies_to=('dts')),
>  ]

[ ... ]

> diff --git a/scripts/dtc/dt-style-selftest/bad/dts-line-length.dts b/scripts/dtc/dt-style-selftest/bad/dts-line-length.dts
> new file mode 100644
> index 0000000000000..d1dacc5c76fbf
> --- /dev/null
> +++ b/scripts/dtc/dt-style-selftest/bad/dts-line-length.dts
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +/*
> + * Test fixture: Unused label in DTS
> + */

[Severity: Low]
This isn't a bug, but does this comment accurately describe the test fixture? 
It appears it might have been copy-pasted from the unused-label test, since 
this file is testing line lengths.

> +
> +/dts-v1/;
> +
> +/ {
> +	soc@0 {
> +		compatible = "simple-bus";

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726-n-dts-style-checker-continued-v1-0-b05b8f897670@oss.qualcomm.com?part=8