Re: [PATCH v13 1/2] dt-bindings: embedded-controller: Add synology microp devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Chronological fallback inversions for older models in DT bindings
- [Medium] Model prefix typo (DS vs RS) for `synology,ds1221rpp-microp`
--

commit 420a38b2f09dfb652dcda0d1d821575521540bff
Author: Markus Probst <markus.probst@posteo.de>

dt-bindings: embedded-controller: Add synology microp devices

This commit adds device tree bindings for Synology Microcontrollers (Microp).
These devices manage LEDs, fans, beepers, and handle power events via a
serial connection on Synology NAS devices.

> diff --git a/Documentation/devicetree/bindings/embedded-controller/synology,ds918p-microp.yaml b/Documentation/devicetree/bindings/embedded-controller/synology,ds918p-microp.yaml
> new file mode 100644
> index 0000000000000..eb1c9d3fac074
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/embedded-controller/synology,ds918p-microp.yaml

[ ... ]

> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - synology,ds1221rpp-microp

[Severity: Medium]
Is synology,ds1221rpp-microp a typo? Synology models with redundant power
supplies are typically RackStations (RS) rather than DiskStations (DS), and
this falls back to synology,rs1221p-microp.

Should this be synology,rs1221rpp-microp instead to avoid establishing a typo
in the permanent device tree ABI?

> +              - synology,rs822p-microp
> +              - synology,rs422p-microp
> +          - const: synology,rs1221p-microp
> +
> +      - items:
> +          - const: synology,ds1825p-microp
> +          - const: synology,ds1823xsp-microp
> +
> +      - items:
> +          - enum:
> +              - synology,ds725p-microp
> +              - synology,ds225p-microp
> +              - synology,ds723p-microp
> +              - synology,ds710p-microp
> +          - const: synology,ds214play-microp

[Severity: Medium]
Is it correct for the older 2010 model synology,ds710p-microp to fall back to
the newer 2014 model synology,ds214play-microp?

In Device Tree schemas, newer hardware variants typically fall back to older
established base models if they are backward compatible, rather than the
other way around.

> +
> +      - items:
> +          - enum:
> +              - synology,ds425p-microp
> +              - synology,ds1525p-microp
> +              - synology,ds925p-microp
> +              - synology,ds923p-microp
> +              - synology,ds1522p-microp
> +              - synology,ds1010p-microp
> +              - synology,ds411p-microp
> +          - const: synology,ds918p-microp

[Severity: Medium]
Does this correctly map the hardware relationships? The 2010 and 2011 models
(synology,ds1010p-microp and synology,ds411p-microp) are configured to fall
back to the 2018 model synology,ds918p-microp.

Since synology,ds918p-microp enforces fan-failure-gpios as a required
property in the conditional block further down in the file, this inverted
fallback forces the older hardware to satisfy constraints defined for newer
hardware. This will cause validation failures for correct older device trees.

Should these older models be listed as their own base models or fall back to
an older base instead?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530-synology_microp_initial-v13-0-aa2bb9c56efc@posteo.de?part=1