Re: [PATCH 04/15] dt-bindings: clock: mediatek: regroup MT8195 dt-bindings into MT8186

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The DT binding schema incorrectly uses `enum` instead of `contains: enum` to match `compatible` strings inside conditional blocks, neutralizing the constraints.
- [Medium] The `else` block restricting reset capabilities incorrectly targets the nonexistent property `reset-cells` instead of `#reset-cells`.
--

commit 5c3ff57e342863b74b59fdf9d908f09394d8d10f
Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

dt-bindings: clock: mediatek: regroup MT8195 dt-bindings into MT8186
    
This commit regroups the MT8195 clock and system clock device tree bindings
into the MT8186 binding files. It adds the MT8195 compatible strings to
mediatek,mt8186-clock.yaml and mediatek,mt8186-sys-clock.yaml, removing the
original MT8195-specific binding files to ease maintainability.

> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt8186-clock.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt8186-clock.yaml
> index 3b543c810f18d..84e602c7d3266 100644
> --- a/Documentation/devicetree/bindings/clock/mediatek,mt8186-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/mediatek,mt8186-clock.yaml
[ ... ]

[Severity: Medium]
While reviewing this file, I noticed an issue in the allOf block introduced
earlier in this series:

allOf:
  - if:
      properties:
        compatible:
          enum:
            - mediatek,mt8188-adsp-audio26m

Does this incorrectly use enum instead of contains: enum?

The schema validation tool treats the compatible property as a string array.
Because enum strictly expects a single string, it will never match an array,
causing the if condition to always evaluate to false.

As a result, the then clauses are skipped entirely, allowing device trees
missing the required #clock-cells and #reset-cells properties to silently
pass schema validation for MT8188 and MT8189 compatibles, which defeats the
purpose of the conditional checks.


[Severity: Medium]
Further down in the same allOf section:

    then:
      required:
        - '#reset-cells'
    else:
      properties:
        reset-cells: false

Does this else block restrict the wrong property name?

It targets the nonexistent property reset-cells (without quotes) instead of
the standard dt-binding property '#reset-cells'.

This allows compatibles that do not support reset capabilities to erroneously
include the #reset-cells property without causing a schema validation failure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-mt8189-clocks-system-base-v1-0-2b048feea50a@collabora.com?part=4