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

[Markus Probst]

On Sun, 2026-04-12 at 10:26 +0200, Krzysztof Kozlowski wrote:
> On Sat, Apr 11, 2026 at 05:27:35PM +0200, Markus Probst wrote:
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - synology,ds923p-microp
> > +      - synology,ds918p-microp
> > +      - synology,ds214play-microp
> > +      - synology,ds225p-microp
> > +      - synology,ds425p-microp
> > +      - synology,ds710p-microp
> > +      - synology,ds1010p-microp
> > +      - synology,ds723p-microp
> > +      - synology,ds1522p-microp
> > +      - synology,rs422p-microp
> > +      - synology,ds725p-microp
> > +      - synology,ds118-microp
> > +      - synology,ds124-microp
> > +      - synology,ds223-microp
> > +      - synology,ds223j-microp
> > +      - synology,ds1823xsp-microp
> > +      - synology,rs822p-microp
> > +      - synology,rs1221p-microp
> > +      - synology,rs1221rpp-microp
> > +      - synology,ds925p-microp
> > +      - synology,ds1525p-microp
> > +      - synology,ds1825p-microp
> 
> Previous comment is not resolved. For example you stated that ds723p is
> compatible with ds725p, so this should be expressed.
Using this expression?

properties:
  compatible:
    oneOf:
      - enum:
          - synology,ds923p-microp
          - synology,ds1522p-microp
      - enum:
          - synology,ds918p-microp
          - synology,ds415p-microp
      - const: synology,ds214play-microp
...
?
If so shall there each be a description?

Also ds723p and ds725p are not compatible. ds723p has a system current
sensor, ds725p does not. This will be relevant when implementing the
hwmon part of the driver.

> 
> ds918p and ds415p as well. ds925p and several others you EXPLICITLY
> wrote they are compatible:
> 
> "ds925p, ds1525p, ds1825p, ds1823xsp:
> - supports fan rpm report via an adt7475 chip and therefore does not
> have gpios for fan failure
> - no system current sensor"
Yes.

> 
> Probably many more cases, I did not verify all of them.
> 
> If there is going to be new version, please organize the patch
> documenting the compatible (DT bindings) before the patch using that
> compatible.
> See also: https://elixir.bootlin.com/linux/v6.14-rc6/source/Documentation/devicetree/bindings/submitting-patches.rst#L46
Ok.

Thanks
- Markus Probst

> 
> > +
> > +  fan-failure-gpios:
> > +    description: GPIOs needed to determine which fans stopped working on a fan failure event.
> > +    minItems: 2
> > +    maxItems: 3
> > +
> > +required:
> > +  - compatible
> 
> Best regards,
> Krzysztof

Attachment: signature.asc
Description: This is a digitally signed message part