public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Markus Probst <markus.probst@posteo.de>
Cc: "Hans de Goede" <hansg@kernel.org>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"Bryan O'Donoghue" <bryan.odonoghue@linaro.org>,
	"Lee Jones" <lee@kernel.org>, "Pavel Machek" <pavel@kernel.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	platform-driver-x86@vger.kernel.org, linux-leds@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	rust-for-linux@vger.kernel.org
Subject: Re: [PATCH v8 1/2] dt-bindings: embedded-controller: Add synology microp devices
Date: Tue, 21 Apr 2026 17:32:14 +0200	[thread overview]
Message-ID: <c214f270-571c-4440-919e-99fce5ac1b08@kernel.org> (raw)
In-Reply-To: <26b074972581ff398b5af964ba092c8117855062.camel@posteo.de>

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.



> 
> 
> 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.

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

>>
>>> +            - 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.

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.

Best regards,
Krzysztof

  reply	other threads:[~2026-04-21 15:32 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-20 14:24 [PATCH v8 0/2] Introduce Synology Microp driver Markus Probst
2026-04-20 14:24 ` [PATCH v8 1/2] dt-bindings: embedded-controller: Add synology microp devices Markus Probst
2026-04-21  7:07   ` Krzysztof Kozlowski
2026-04-21 14:50     ` Markus Probst
2026-04-21 15:32       ` Krzysztof Kozlowski [this message]
2026-04-21 16:25         ` Markus Probst
2026-04-23  9:38           ` Krzysztof Kozlowski
2026-04-20 14:24 ` [PATCH v8 2/2] platform: Add initial synology microp driver Markus Probst
2026-04-21 11:59   ` Ilpo Järvinen
2026-04-21 14:17     ` Markus Probst
2026-04-21 14:58       ` Miguel Ojeda
2026-04-21 18:10       ` Ilpo Järvinen
2026-04-21 18:20         ` Markus Probst
2026-04-21 18:36           ` Ilpo Järvinen
2026-04-21 18:46           ` Miguel Ojeda
2026-04-22 13:48             ` FUJITA Tomonori
2026-04-21 15:33   ` Krzysztof Kozlowski
2026-04-21 16:29     ` Markus Probst
2026-04-20 15:55 ` [PATCH v8 0/2] Introduce Synology Microp driver Markus Probst

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c214f270-571c-4440-919e-99fce5ac1b08@kernel.org \
    --to=krzk@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=bryan.odonoghue@linaro.org \
    --cc=conor+dt@kernel.org \
    --cc=dakr@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=hansg@kernel.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=krzk+dt@kernel.org \
    --cc=lee@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=markus.probst@posteo.de \
    --cc=ojeda@kernel.org \
    --cc=pavel@kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox