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

[Markus Probst <markus.probst@posteo.de>]

[-- Attachment #1: Type: text/plain, Size: 6526 bytes --]

On Tue, 2026-04-21 at 17:32 +0200, Krzysztof Kozlowski wrote:
> On 21/04/2026 16:50, Markus Probst wrote:
> > On Tue, 2026-04-21 at 09:07 +0200, Krzysztof Kozlowski wrote:
> > > On Mon, Apr 20, 2026 at 02:24:20PM +0000, Markus Probst wrote:
> > > > Add the Synology Microp devicetree bindings. Those devices are
> > > > microcontrollers found on Synology NAS devices. They are connected to a
> > > > serial port on the host device.
> > > > 
> > > > Those devices are used to control certain LEDs, fan speeds, a beeper, to
> > > > handle buttons, fan failures and to properly shutdown and reboot the
> > > > device.
> > > > 
> > > > The device has a different feature set depending on the Synology NAS
> > > > model, like having different number of fans, buttons and leds. Depending
> > > > on the architecture of the model, they also need a different system
> > > > shutdown behaviour.
> > > > 
> > > > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > > > ---
> > > >  .../synology,ds1825p-microp.yaml                   | 108 +++++++++++++++++++++
> > > >  1 file changed, 108 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/embedded-controller/synology,ds1825p-microp.yaml b/Documentation/devicetree/bindings/embedded-controller/synology,ds1825p-microp.yaml
> > > > new file mode 100644
> > > > index 000000000000..76c671a42fbf
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/embedded-controller/synology,ds1825p-microp.yaml
> > > > @@ -0,0 +1,108 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/embedded-controller/synology,ds1825p-microp.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Synology NAS on-board Microcontroller
> > > > +
> > > > +maintainers:
> > > > +  - Markus Probst <markus.probst@posteo.de>
> > > > +
> > > > +description: |
> > > > +  Synology Microp is a microcontroller found in Synology NAS devices.
> > > > +  It is connected to a serial port on the host device.
> > > > +
> > > > +  It is necessary to properly shutdown and reboot the NAS device and
> > > > +  provides additional functionality such as led control, fan speed control,
> > > > +  a beeper and buttons on the NAS device.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    oneOf:
> > > > +      - const: synology,ds223-microp
> > > > +      - const: synology,ds411p-microp
> > > > +      - const: synology,ds1010p-microp
> > > > +      - const: synology,ds710p-microp
> > > > +      - const: synology,ds723p-microp
> > > > +      - const: synology,ds225p-microp
> > > > +      - const: synology,rs422p-microp
> > > 
> > > That's one enum.
> > > 
> > > > +      - maxItems: 2
> > > > +        minItems: 2
> > > 
> > > There is no such syntax foro compatibles. Please use any existing file
> > > as example or look at example-schema.
> > In the example schema, another device is used as fallback. 
> 
> True.
> 
> > This is what
> > I did here.
> 
> Not true. You have enum and min/maxItems. There is no such syntax for
> compatibles. I repeat.
> 
> Instead of just blindly disagreeing and saying "I did that", point me to
> example-schema having compatibles with min/maxItems.
It is supposed to minimize the schema, otherwise

- minItems: 3
  maxItems: 3
  items:
    enum:
    - synology,ds223j-microp
    - synology,ds124-microp
    - synology,ds118-microp

would have been

- items:
    - const: synology,ds223j-microp
    - const: synology,ds124-microp
    - const: synology,ds118-microp

- items:
    - const: synology,ds124-microp
    - const: synology,ds223j-microp
    - const: synology,ds118-microp

- items:
    - const: synology,ds118-microp
    - const: synology,ds223j-microp
    - const: synology,ds124-microp
.

In the case of dts validation, this is the same, with the exception of
allowing any order. See below why the order does not matter.

And yes, the example schema didn't use min/maxItems.
> 
> 
> 
> > 
> > 
> > Other sources suggest, I should add fallbacks that are less specific
> 
> That's not really discussed here. It all looks like some random schema
> and considering amount of LLM flying on the lists I have now doubts.
I am not using an LLM for coding. But it is the first device tree
schema I try to write.
> 
> You need specific compatibles.
> ...
> 
> > 
> > If thisisn't fine either, replying to my previous message would
> > probably the most efficient way to move forward [1].
> 
> 
> > 
> > > 
> > > > +        items:
> > > > +          enum:
> > > 
> > > No, why the list is randomly ordered.
> 
> Look here
Was answered below. They have the exactly same known feature set, thus
the order does not matter. i. e. there is no known difference.

> 
> > > 
> > > > +            - synology,ds923p-microp
> > > > +            - synology,ds1522p-microp
> > > 
> > > And fallback, whichever is that, is not documented alone.
> > > 
> > > > +      - minItems: 4
> > > > +        maxItems: 4
> > 
> > Those are devices with the exactly same known feature set.
> > i. e. ds1522p can act as a fallback for ds923p, and ds923p could act as
> > a fallback for ds1522p.
> 
> You are not responding to actual comments. Lets focus ONLY on above
> list. ONLY. Point me, where did you document the fallback to be used
> alone? First of course, define what is the fallback.
In this schema, I defined ds923p as fallback for ds1522p, and ds1522p
as fallback for ds923p. There was no separation of front and fallback
until now. See below.

> 
> None of this matches example schema or any other bindings, none of this
> produces correct constraints for correct DTS.
> 
> You need a defined enum of fallbacks and several lists for specific
> fallback+front, like many other bindings in kernel.
So now I can imagine what schema you want to see.

i. e.
- items:
    - enum:
      - front1
      - front2
      - front3
    - const: fallback1
- items:
    - enum:
      - front4
      - front5
      - front6
    - const: fallback2
- enum:
  - fallback1
  - fallback2

But I am unsure how to determine which devices are the fallback and
which are the front devices, given some of them have the same feature
set. The oldest device as fallback each?

Thanks
- Markus Probst

> 
> Best regards,
> Krzysztof

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]