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

[Krzysztof Kozlowski]

On 06/04/2026 16:22, Markus Probst wrote:
> On Mon, 2026-04-06 at 09:59 +0200, Krzysztof Kozlowski wrote:
>> On Sun, Apr 05, 2026 at 07:36:29PM +0200, 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.
>>>
>>> This includes the following compatible ids:
>>>  - 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
>>
>> Drop, we see this in the diff.
> A prior review commit suggested I should add them [1].
> So only synology,ds923p-microp in the Subject then?

I do not see how this list resolves my comment. Really, explain my how
listing part of binding answers WHY they are not compatible?


> 
> [1]
> https://lore.kernel.org/all/20260330-delicate-sassy-mayfly-ebcca7@quoll/
> 
>>
>>>
>>> Signed-off-by: Markus Probst <markus.probst@xxxxxxxxx>
>>> ---
>>>  .../synology,ds923p-microp.yaml                    | 112 +++++++++++++++++++++
>>>  MAINTAINERS                                        |   1 +
>>>  2 files changed, 113 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/embedded-controller/synology,ds923p-microp.yaml b/Documentation/devicetree/bindings/embedded-controller/synology,ds923p-microp.yaml
>>> new file mode 100644
>>> index 000000000000..4518e9b74be1
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/embedded-controller/synology,ds923p-microp.yaml
>>> @@ -0,0 +1,112 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/embedded-controller/synology,ds923p-microp.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Synology NAS on-board Microcontroller
>>> +
>>> +maintainers:
>>> +  - Markus Probst <markus.probst@xxxxxxxxx>
>>> +
>>> +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:
>>> +    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
>>
>> So we already talked about this and you were told to use compatibility.
>> Your driver clearly states several of these are compatible, so I am
>> confused that I do not see it expressed here.
> The driver does not have all functionality implemented yet.

Either this drivers works or not. If it works, explain me how they are
not compatible.

> 
> A few examples of differences not yet visible in the driver:
> - synology,ds214play-microp is the only model in the current list to
> have an cpu fan
> - 4 of the models are arm based and need a different shutdown behaviour
> - different amount of fans (already present in the binding via fan-
> failure-gpios)
> 
> I could try to group them together, but Synology does not document the
> exact difference between them.
> 
> As Rob mentioned [2], I need to be able to handle unexpected
> differences without qurik properties.

I did not object that.


Best regards,
Krzysztof