* [PATCH v8 0/2] Introduce Synology Microp driver
@ 2026-04-20 14:24 Markus Probst
2026-04-20 14:24 ` [PATCH v8 1/2] dt-bindings: embedded-controller: Add synology microp devices Markus Probst
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Markus Probst @ 2026-04-20 14:24 UTC (permalink / raw)
To: Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman
Cc: platform-driver-x86, linux-leds, devicetree, linux-kernel,
rust-for-linux, Markus Probst
Synology uses a microcontroller in their NAS devices connected to a
serial port to control certain LEDs, fan speeds, a beeper, to handle
proper shutdown and restart, buttons and fan failures.
This patch series depends on the rust led abstraction [1] and the rust
serdev abstraction [2].
This is only a initial version of the driver able to control LEDs.
The following rust abstractions would be required, to implement the
remaining features:
- hwmon (include/linux/hwmon.h)
- input (include/linux/input.h)
- sysoff handler + hardware protection shutdown (include/linux/reboot.h)
[1] https://lore.kernel.org/rust-for-linux/20260329-rust_leds-v13-0-21a599c5b2d1@posteo.de/
[2] https://lore.kernel.org/rust-for-linux/20260411-rust_serdev-v4-0-845e960c6627@posteo.de/
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
Changes in v8:
- removed unnecessary Copy and Clone derive
- added `BLINK_DELAY` constant
- added compatible id fallbacks
- moved dt schema patch before the driver
- added ds411p
- Link to v7: https://lore.kernel.org/r/20260411-synology_microp_initial-v7-0-9a3a094e763a@posteo.de
Changes in v7:
- remove list of compatible ids from commit msg
- explain what makes the different models not compatible in the commit msg
- remove unnecessary examples
- Link to v6: https://lore.kernel.org/r/20260405-synology_microp_initial-v6-0-08fde474b6c9@posteo.de
Changes in v6:
- moved devicetree bindings patch at the end of the set
- remove several patches
- move of id table from model.rs to synology_microp.rs
- remove the model! macro
- use if blocks in devicetree schema to narrow down the
fan-failure-gpios property
- add multiple devicetree examples to test if blocks
- Link to v5: https://lore.kernel.org/r/20260329-synology_microp_initial-v5-0-27cb80bdf591@posteo.de
Changes in v5:
- add esata led support
- use different compatible for each model
- add visibility modifier to of_device_table macro
- fix match data missing when using PRP0001
- Link to v4: https://lore.kernel.org/r/20260320-synology_microp_initial-v4-0-0423ddb83ca4@posteo.de
Changes in v4:
- convert to monolithic driver and moved it into drivers/platform
- removed mfd rust abstraction
- moved dt-bindings to embedded-controller
- Link to v3: https://lore.kernel.org/r/20260313-synology_microp_initial-v3-0-ad6ac463a201@posteo.de
Changes in v3:
- remove `default n` from Kconfig entry, as n is the default already.
- select RUST_SERIAL_DEV_BUS_ABSTRACTIONS in Kconfig
- add mfd rust abstraction
- split core and led parts into their own driver. It should now be considered a
MFD device.
- split led part of dt binding into its own file
- Link to v2: https://lore.kernel.org/r/20260308-synology_microp_initial-v2-0-9389963f31c5@posteo.de
Changes in v2:
- fix missing tabs in MAINTAINERS file
- remove word binding from patch subject
- add missing signed-off-by
- add missing help entry in Kconfig
- add missing spdx license headers
- remove no-check{,-cpu}-fan properties from the dt-bindings and replace
them with the check_fan module parameter
- use patternProperties for leds in dt-bindings
- license dt-binding as GPL-2.0-only OR BSD-2-Clause
- move driver from staging tree into mfd tree and mark it as work in
progress inside Kconfig
- only register alert and usb led if fwnode is present
- Link to v1: https://lore.kernel.org/r/20260306-synology_microp_initial-v1-0-fcffede6448c@posteo.de
To: Markus Probst <markus.probst@posteo.de>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Miguel Ojeda <ojeda@kernel.org>
To: Boqun Feng <boqun@kernel.org>
To: Gary Guo <gary@garyguo.net>
To: Björn Roy Baron <bjorn3_gh@protonmail.com>
To: Benno Lossin <lossin@kernel.org>
To: Andreas Hindborg <a.hindborg@kernel.org>
To: Alice Ryhl <aliceryhl@google.com>
To: Trevor Gross <tmgross@umich.edu>
To: Danilo Krummrich <dakr@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: rust-for-linux@vger.kernel.org
---
Markus Probst (2):
dt-bindings: embedded-controller: Add synology microp devices
platform: Add initial synology microp driver
.../synology,ds1825p-microp.yaml | 108 ++++++++
MAINTAINERS | 6 +
drivers/platform/Kconfig | 2 +
drivers/platform/Makefile | 1 +
drivers/platform/synology_microp/Kconfig | 13 +
drivers/platform/synology_microp/Makefile | 3 +
drivers/platform/synology_microp/TODO | 7 +
drivers/platform/synology_microp/command.rs | 54 ++++
drivers/platform/synology_microp/led.rs | 281 +++++++++++++++++++++
drivers/platform/synology_microp/model.rs | 49 ++++
.../platform/synology_microp/synology_microp.rs | 110 ++++++++
11 files changed, 634 insertions(+)
---
base-commit: 3131ff5a117498bb4b9db3a238bb311cbf8383ce
change-id: 20260306-synology_microp_initial-0f7dac7b7496
prerequisite-change-id: 20251217-rust_serdev-ee5481e9085c:v4
prerequisite-patch-id: 52b17274481cc770c257d8f95335293eca32a2c5
prerequisite-patch-id: eec47e5051640d08bcd34a9670b98804449cad52
prerequisite-patch-id: f24b68c71c3f69371e8ac0251efca0a023b31cc4
prerequisite-patch-id: d0686cf451ef899a06d468adfba51ccd84e6ff98
prerequisite-change-id: 20251114-rust_leds-a959f7c2f7f9:v13
prerequisite-patch-id: 818700f22dcb9676157c985f82762d7c607b861e
prerequisite-patch-id: b15ffa7d95d9260151bfb116b259c4473f721c82
prerequisite-patch-id: 8c47e0d107530f577a1be0b79f8ee791f95d3cbe
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v8 1/2] dt-bindings: embedded-controller: Add synology microp devices
2026-04-20 14:24 [PATCH v8 0/2] Introduce Synology Microp driver Markus Probst
@ 2026-04-20 14:24 ` Markus Probst
2026-04-21 7:07 ` Krzysztof Kozlowski
2026-04-20 14:24 ` [PATCH v8 2/2] platform: Add initial synology microp driver Markus Probst
2026-04-20 15:55 ` [PATCH v8 0/2] Introduce Synology Microp driver Markus Probst
2 siblings, 1 reply; 19+ messages in thread
From: Markus Probst @ 2026-04-20 14:24 UTC (permalink / raw)
To: Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman
Cc: platform-driver-x86, linux-leds, devicetree, linux-kernel,
rust-for-linux, Markus Probst
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
+ - maxItems: 2
+ minItems: 2
+ items:
+ enum:
+ - synology,ds923p-microp
+ - synology,ds1522p-microp
+ - minItems: 4
+ maxItems: 4
+ items:
+ enum:
+ - synology,ds918p-microp
+ - synology,ds425p-microp
+ - synology,ds1525p-microp
+ - synology,ds925p-microp
+ - minItems: 2
+ maxItems: 2
+ items:
+ enum:
+ - synology,ds725p-microp
+ - synology,ds214play-microp
+ - minItems: 3
+ maxItems: 3
+ items:
+ enum:
+ - synology,ds223j-microp
+ - synology,ds124-microp
+ - synology,ds118-microp
+ - minItems: 3
+ maxItems: 3
+ items:
+ enum:
+ - synology,rs822p-microp
+ - synology,rs1221p-microp
+ - synology,rs1221rpp-microp
+ - minItems: 2
+ maxItems: 2
+ items:
+ enum:
+ - synology,ds1825p-microp
+ - synology,ds1823xsp-microp
+
+ fan-failure-gpios:
+ description: GPIOs needed to determine which fans stopped working on a fan failure event.
+ minItems: 2
+ maxItems: 3
+
+required:
+ - compatible
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - synology,ds425p-microp
+ - synology,rs422p-microp
+ - synology,ds1522p-microp
+ - synology,ds1010p-microp
+ - synology,ds411p-microp
+ then:
+ required:
+ - fan-failure-gpios
+ else:
+ properties:
+ fan-failure-gpios: false
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/leds/common.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ embedded-controller {
+ compatible = "synology,ds923p-microp", "synology,ds1522p-microp";
+
+ fan-failure-gpios = <&gpio 68 GPIO_ACTIVE_HIGH>, <&gpio 69 GPIO_ACTIVE_HIGH>;
+ };
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 2/2] platform: Add initial synology microp driver
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-20 14:24 ` Markus Probst
2026-04-21 11:59 ` Ilpo Järvinen
2026-04-21 15:33 ` Krzysztof Kozlowski
2026-04-20 15:55 ` [PATCH v8 0/2] Introduce Synology Microp driver Markus Probst
2 siblings, 2 replies; 19+ messages in thread
From: Markus Probst @ 2026-04-20 14:24 UTC (permalink / raw)
To: Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman
Cc: platform-driver-x86, linux-leds, devicetree, linux-kernel,
rust-for-linux, Markus Probst
Add a initial synology microp driver, written in Rust.
The driver targets a microcontroller found in Synology NAS devices. It
currently only supports controlling of the power led, status led, alert
led and usb led. Other components such as fan control or handling
on-device buttons will be added once the required rust abstractions are
there.
This driver can be used both on arm and x86, thus it goes into the root
directory of drivers/platform.
Tested successfully on a Synology DS923+.
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
MAINTAINERS | 6 +
drivers/platform/Kconfig | 2 +
drivers/platform/Makefile | 1 +
drivers/platform/synology_microp/Kconfig | 13 +
drivers/platform/synology_microp/Makefile | 3 +
drivers/platform/synology_microp/TODO | 7 +
drivers/platform/synology_microp/command.rs | 54 ++++
drivers/platform/synology_microp/led.rs | 281 +++++++++++++++++++++
drivers/platform/synology_microp/model.rs | 49 ++++
.../platform/synology_microp/synology_microp.rs | 110 ++++++++
10 files changed, 526 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index c1c686846cdd..49f08290eed0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25555,6 +25555,12 @@ F: drivers/dma-buf/sync_*
F: include/linux/sync_file.h
F: include/uapi/linux/sync_file.h
+SYNOLOGY MICROP DRIVER
+M: Markus Probst <markus.probst@posteo.de>
+S: Maintained
+F: Documentation/devicetree/bindings/embedded-controller/synology,ds1825p-microp.yaml
+F: drivers/platform/synology_microp/
+
SYNOPSYS ARC ARCHITECTURE
M: Vineet Gupta <vgupta@kernel.org>
L: linux-snps-arc@lists.infradead.org
diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
index 312788f249c9..996050566a4a 100644
--- a/drivers/platform/Kconfig
+++ b/drivers/platform/Kconfig
@@ -22,3 +22,5 @@ source "drivers/platform/arm64/Kconfig"
source "drivers/platform/raspberrypi/Kconfig"
source "drivers/platform/wmi/Kconfig"
+
+source "drivers/platform/synology_microp/Kconfig"
diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
index fa322e7f8716..2381872e9133 100644
--- a/drivers/platform/Makefile
+++ b/drivers/platform/Makefile
@@ -15,3 +15,4 @@ obj-$(CONFIG_SURFACE_PLATFORMS) += surface/
obj-$(CONFIG_ARM64_PLATFORM_DEVICES) += arm64/
obj-$(CONFIG_BCM2835_VCHIQ) += raspberrypi/
obj-$(CONFIG_ACPI_WMI) += wmi/
+obj-$(CONFIG_SYNOLOGY_MICROP) += synology_microp/
diff --git a/drivers/platform/synology_microp/Kconfig b/drivers/platform/synology_microp/Kconfig
new file mode 100644
index 000000000000..7c4d8f2808f0
--- /dev/null
+++ b/drivers/platform/synology_microp/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config SYNOLOGY_MICROP
+ tristate "Synology Microp driver"
+ depends on LEDS_CLASS && LEDS_CLASS_MULTICOLOR
+ depends on RUST_SERIAL_DEV_BUS_ABSTRACTIONS
+ help
+ Enable support for the MCU found in Synology NAS devices.
+
+ This is needed to properly shutdown and reboot the device, as well as
+ additional functionality like fan and LED control.
+
+ This driver is work in progress and may not be fully functional.
diff --git a/drivers/platform/synology_microp/Makefile b/drivers/platform/synology_microp/Makefile
new file mode 100644
index 000000000000..63585ccf76e4
--- /dev/null
+++ b/drivers/platform/synology_microp/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-y += synology_microp.o
diff --git a/drivers/platform/synology_microp/TODO b/drivers/platform/synology_microp/TODO
new file mode 100644
index 000000000000..1961a33115db
--- /dev/null
+++ b/drivers/platform/synology_microp/TODO
@@ -0,0 +1,7 @@
+TODO:
+- add missing components:
+ - handle on-device buttons (Power, Factory reset, "USB Copy")
+ - handle fan failure
+ - beeper
+ - fan speed control
+ - correctly perform device power-off and restart on Synology devices
diff --git a/drivers/platform/synology_microp/command.rs b/drivers/platform/synology_microp/command.rs
new file mode 100644
index 000000000000..430cb858e1c3
--- /dev/null
+++ b/drivers/platform/synology_microp/command.rs
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use kernel::{
+ device::Bound,
+ error::Result,
+ serdev, //
+};
+
+use crate::led;
+
+#[expect(
+ clippy::enum_variant_names,
+ reason = "future variants will not end with Led"
+)]
+pub(crate) enum Command {
+ PowerLed(led::State),
+ StatusLed(led::StatusLedColor, led::State),
+ AlertLed(led::State),
+ UsbLed(led::State),
+ EsataLed(led::State),
+}
+
+impl Command {
+ pub(crate) fn write(self, dev: &serdev::Device<Bound>) -> Result {
+ dev.write_all(
+ match self {
+ Self::PowerLed(led::State::On) => &[0x34],
+ Self::PowerLed(led::State::Blink) => &[0x35],
+ Self::PowerLed(led::State::Off) => &[0x36],
+
+ Self::StatusLed(_, led::State::Off) => &[0x37],
+ Self::StatusLed(led::StatusLedColor::Green, led::State::On) => &[0x38],
+ Self::StatusLed(led::StatusLedColor::Green, led::State::Blink) => &[0x39],
+ Self::StatusLed(led::StatusLedColor::Orange, led::State::On) => &[0x3A],
+ Self::StatusLed(led::StatusLedColor::Orange, led::State::Blink) => &[0x3B],
+
+ Self::AlertLed(led::State::On) => &[0x4C, 0x41, 0x31],
+ Self::AlertLed(led::State::Blink) => &[0x4C, 0x41, 0x32],
+ Self::AlertLed(led::State::Off) => &[0x4C, 0x41, 0x33],
+
+ Self::UsbLed(led::State::On) => &[0x40],
+ Self::UsbLed(led::State::Blink) => &[0x41],
+ Self::UsbLed(led::State::Off) => &[0x42],
+
+ Self::EsataLed(led::State::On) => &[0x4C, 0x45, 0x31],
+ Self::EsataLed(led::State::Blink) => &[0x4C, 0x45, 0x32],
+ Self::EsataLed(led::State::Off) => &[0x4C, 0x45, 0x33],
+ },
+ serdev::Timeout::Max,
+ )?;
+ dev.wait_until_sent(serdev::Timeout::Max);
+ Ok(())
+ }
+}
diff --git a/drivers/platform/synology_microp/led.rs b/drivers/platform/synology_microp/led.rs
new file mode 100644
index 000000000000..f89998a7e6b4
--- /dev/null
+++ b/drivers/platform/synology_microp/led.rs
@@ -0,0 +1,281 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use kernel::{
+ device::Bound,
+ devres::{
+ self,
+ Devres, //
+ },
+ led::{
+ self,
+ LedOps,
+ MultiColorSubLed, //
+ },
+ new_mutex,
+ prelude::*,
+ serdev,
+ str::CString,
+ sync::Mutex, //
+};
+use pin_init::pin_init_scope;
+
+use crate::{
+ command::Command,
+ model::Model, //
+};
+
+#[pin_data]
+pub(crate) struct Data {
+ #[pin]
+ status: Devres<led::MultiColorDevice<StatusLedHandler>>,
+ power_name: CString,
+ #[pin]
+ power: Devres<led::Device<LedHandler>>,
+}
+
+impl Data {
+ pub(super) fn register<'a>(
+ dev: &'a serdev::Device<Bound>,
+ model: &'a Model,
+ ) -> impl PinInit<Self, Error> + 'a {
+ pin_init_scope(move || {
+ if let Some(color) = model.led_alert {
+ let name = CString::try_from_fmt(fmt!("{}:alarm", color.as_c_str().to_str()?))?;
+ devres::register(
+ dev.as_ref(),
+ led::DeviceBuilder::new().color(color).name(&name).build(
+ dev,
+ try_pin_init!(LedHandler {
+ blink <- new_mutex!(false),
+ command: Command::AlertLed,
+ }),
+ ),
+ GFP_KERNEL,
+ )?;
+ }
+
+ if model.led_usb_copy {
+ devres::register(
+ dev.as_ref(),
+ led::DeviceBuilder::new()
+ .color(led::Color::Green)
+ .name(c"green:usb")
+ .build(
+ dev,
+ try_pin_init!(LedHandler {
+ blink <- new_mutex!(false),
+ command: Command::UsbLed,
+ }),
+ ),
+ GFP_KERNEL,
+ )?;
+ }
+
+ if model.led_esata {
+ devres::register(
+ dev.as_ref(),
+ led::DeviceBuilder::new()
+ .color(led::Color::Green)
+ .name(c"green:esata")
+ .build(
+ dev,
+ try_pin_init!(LedHandler {
+ blink <- new_mutex!(false),
+ command: Command::EsataLed,
+ }),
+ ),
+ GFP_KERNEL,
+ )?;
+ }
+
+ Ok(try_pin_init!(Self {
+ status <- led::DeviceBuilder::new()
+ .color(led::Color::Multi)
+ .name(c"multicolor:status")
+ .build_multicolor(
+ dev,
+ try_pin_init!(StatusLedHandler {
+ blink <- new_mutex!(false),
+ }),
+ StatusLedHandler::SUBLEDS,
+ ),
+ power_name: CString::try_from_fmt(fmt!(
+ "{}:power",
+ model.led_power.as_c_str().to_str()?
+ ))?,
+ power <- led::DeviceBuilder::new()
+ .color(model.led_power)
+ .name(power_name)
+ .build(
+ dev,
+ try_pin_init!(LedHandler {
+ blink <- new_mutex!(true),
+ command: Command::PowerLed,
+ }),
+ ),
+ }))
+ })
+ }
+}
+
+#[derive(Copy, Clone)]
+pub(crate) enum StatusLedColor {
+ Green,
+ Orange,
+}
+
+#[derive(Copy, Clone)]
+pub(crate) enum State {
+ On,
+ Blink,
+ Off,
+}
+
+#[pin_data]
+struct LedHandler {
+ #[pin]
+ blink: Mutex<bool>,
+ command: fn(State) -> Command,
+}
+
+/// Blink delay measured using video recording on DS923+ for Power and Status Led.
+///
+/// We assume it is the same for all other leds and models.
+const BLINK_DELAY: usize = 167;
+
+#[vtable]
+impl LedOps for LedHandler {
+ type Bus = serdev::Device<Bound>;
+ type Mode = led::Normal;
+ const BLOCKING: bool = true;
+ const MAX_BRIGHTNESS: u32 = 1;
+
+ fn brightness_set(
+ &self,
+ dev: &Self::Bus,
+ _classdev: &led::Device<Self>,
+ brightness: u32,
+ ) -> Result<()> {
+ let mut blink = self.blink.lock();
+ (self.command)(if brightness == 0 {
+ *blink = false;
+ State::Off
+ } else if *blink {
+ State::Blink
+ } else {
+ State::On
+ })
+ .write(dev)?;
+
+ Ok(())
+ }
+
+ fn blink_set(
+ &self,
+ dev: &Self::Bus,
+ _classdev: &led::Device<Self>,
+ delay_on: &mut usize,
+ delay_off: &mut usize,
+ ) -> Result<()> {
+ let mut blink = self.blink.lock();
+
+ (self.command)(if *delay_on == 0 && *delay_off != 0 {
+ State::Off
+ } else if *delay_on != 0 && *delay_off == 0 {
+ State::On
+ } else {
+ *blink = true;
+ *delay_on = BLINK_DELAY;
+ *delay_off = BLINK_DELAY;
+
+ State::Blink
+ })
+ .write(dev)
+ }
+}
+
+#[pin_data]
+struct StatusLedHandler {
+ #[pin]
+ blink: Mutex<bool>,
+}
+
+impl StatusLedHandler {
+ const SUBLEDS: &[MultiColorSubLed] = &[
+ MultiColorSubLed::new(led::Color::Green).initial_intensity(1),
+ MultiColorSubLed::new(led::Color::Orange),
+ ];
+}
+
+#[vtable]
+impl LedOps for StatusLedHandler {
+ type Bus = serdev::Device<Bound>;
+ type Mode = led::MultiColor;
+ const BLOCKING: bool = true;
+ const MAX_BRIGHTNESS: u32 = 1;
+
+ fn brightness_set(
+ &self,
+ dev: &Self::Bus,
+ classdev: &led::MultiColorDevice<Self>,
+ brightness: u32,
+ ) -> Result<()> {
+ let mut blink = self.blink.lock();
+ if brightness == 0 {
+ *blink = false;
+ }
+
+ let (color, subled_brightness) = if classdev.subleds()[1].intensity == 0 {
+ (StatusLedColor::Green, classdev.subleds()[0].brightness)
+ } else {
+ (StatusLedColor::Orange, classdev.subleds()[1].brightness)
+ };
+
+ Command::StatusLed(
+ color,
+ if subled_brightness == 0 {
+ State::Off
+ } else if *blink {
+ State::Blink
+ } else {
+ State::On
+ },
+ )
+ .write(dev)
+ }
+
+ fn blink_set(
+ &self,
+ dev: &Self::Bus,
+ classdev: &led::MultiColorDevice<Self>,
+ delay_on: &mut usize,
+ delay_off: &mut usize,
+ ) -> Result<()> {
+ let mut blink = self.blink.lock();
+ *blink = true;
+
+ let (color, subled_intensity) = if classdev.subleds()[1].intensity == 0 {
+ (StatusLedColor::Green, classdev.subleds()[0].intensity)
+ } else {
+ (StatusLedColor::Orange, classdev.subleds()[1].intensity)
+ };
+ Command::StatusLed(
+ color,
+ if *delay_on == 0 && *delay_off != 0 {
+ *blink = false;
+ State::Off
+ } else if subled_intensity == 0 {
+ State::Off
+ } else if *delay_on != 0 && *delay_off == 0 {
+ *blink = false;
+ State::On
+ } else {
+ *delay_on = BLINK_DELAY;
+ *delay_off = BLINK_DELAY;
+
+ State::Blink
+ },
+ )
+ .write(dev)
+ }
+}
diff --git a/drivers/platform/synology_microp/model.rs b/drivers/platform/synology_microp/model.rs
new file mode 100644
index 000000000000..715d8840f56b
--- /dev/null
+++ b/drivers/platform/synology_microp/model.rs
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use kernel::led::Color;
+
+pub(crate) struct Model {
+ pub(crate) led_power: Color,
+ pub(crate) led_alert: Option<Color>,
+ pub(crate) led_usb_copy: bool,
+ pub(crate) led_esata: bool,
+}
+
+impl Model {
+ pub(super) const fn new() -> Self {
+ Self {
+ led_power: Color::Blue,
+ led_alert: None,
+ led_usb_copy: false,
+ led_esata: false,
+ }
+ }
+
+ pub(super) const fn led_power(self, color: Color) -> Self {
+ Self {
+ led_power: color,
+ ..self
+ }
+ }
+
+ pub(super) const fn led_alert(self, color: Color) -> Self {
+ Self {
+ led_alert: Some(color),
+ ..self
+ }
+ }
+
+ pub(super) const fn led_esata(self) -> Self {
+ Self {
+ led_esata: true,
+ ..self
+ }
+ }
+
+ pub(super) const fn led_usb_copy(self) -> Self {
+ Self {
+ led_usb_copy: true,
+ ..self
+ }
+ }
+}
diff --git a/drivers/platform/synology_microp/synology_microp.rs b/drivers/platform/synology_microp/synology_microp.rs
new file mode 100644
index 000000000000..1fd4fc658d85
--- /dev/null
+++ b/drivers/platform/synology_microp/synology_microp.rs
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Synology Microp driver
+
+use kernel::{
+ device,
+ led::Color,
+ of::{
+ DeviceId,
+ IdTable, //
+ },
+ of_device_table,
+ prelude::*,
+ serdev, //
+};
+use pin_init::pin_init_scope;
+
+use crate::model::Model;
+
+pub(crate) mod command;
+mod led;
+mod model;
+
+kernel::module_serdev_device_driver! {
+ type: SynologyMicropDriver,
+ name: "synology_microp",
+ authors: ["Markus Probst <markus.probst@posteo.de>"],
+ description: "Synology Microp driver",
+ license: "GPL v2",
+}
+
+#[rustfmt::skip]
+of_device_table!(
+ OF_TABLE,
+ MODULE_OF_TABLE,
+ Model,
+ [
+ // apollolake
+ (DeviceId::new(c"synology,ds918p-microp"), Model::new()),
+
+ // evansport
+ (DeviceId::new(c"synology,ds214play-microp"), Model::new()),
+
+ // geminilakenk
+ (DeviceId::new(c"synology,ds225p-microp"), Model::new().led_usb_copy()),
+ (DeviceId::new(c"synology,ds425p-microp"), Model::new()),
+
+ // pineview
+ (DeviceId::new(c"synology,ds710p-microp"), Model::new().led_esata()),
+ (DeviceId::new(c"synology,ds1010p-microp"), Model::new().led_alert(Color::Orange)),
+ (DeviceId::new(c"synology,ds411p-microp"), Model::new()),
+
+ // r1000
+ (DeviceId::new(c"synology,ds923p-microp"), Model::new()),
+ (DeviceId::new(c"synology,ds723p-microp"), Model::new()),
+ (DeviceId::new(c"synology,ds1522p-microp"), Model::new()),
+ (DeviceId::new(c"synology,rs422p-microp"), Model::new().led_power(Color::Green)),
+
+ // r1000nk
+ (DeviceId::new(c"synology,ds725p-microp"), Model::new()),
+
+ // rtd1296
+ (DeviceId::new(c"synology,ds118-microp"), Model::new()),
+
+ // rtd1619b
+ (DeviceId::new(c"synology,ds124-microp"), Model::new()),
+ (DeviceId::new(c"synolody,ds223-microp"), Model::new().led_usb_copy()),
+ (DeviceId::new(c"synology,ds223j-microp"), Model::new()),
+
+ // v1000
+ (DeviceId::new(c"synology,ds1823xsp-microp"), Model::new()),
+ (DeviceId::new(c"synology,rs822p-microp"), Model::new().led_power(Color::Green)),
+ (DeviceId::new(c"synology,rs1221p-microp"), Model::new().led_power(Color::Green)),
+ (DeviceId::new(c"synology,rs1221rpp-microp"), Model::new().led_power(Color::Green)),
+
+ // v1000nk
+ (DeviceId::new(c"synology,ds925p-microp"), Model::new()),
+ (DeviceId::new(c"synology,ds1525p-microp"), Model::new()),
+ (DeviceId::new(c"synology,ds1825p-microp"), Model::new()),
+ ]
+);
+
+#[pin_data]
+struct SynologyMicropDriver {
+ #[pin]
+ led: led::Data,
+}
+
+#[vtable]
+impl serdev::Driver for SynologyMicropDriver {
+ type IdInfo = Model;
+ const OF_ID_TABLE: Option<IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+
+ fn probe(
+ dev: &serdev::Device<device::Core>,
+ model: Option<&Model>,
+ ) -> impl PinInit<Self, kernel::error::Error> {
+ pin_init_scope(move || {
+ let model = model.ok_or(EINVAL)?;
+
+ dev.set_baudrate(9600).map_err(|_| EINVAL)?;
+ dev.set_flow_control(false);
+ dev.set_parity(serdev::Parity::None)?;
+
+ Ok(try_pin_init!(Self {
+ led <- led::Data::register(dev, model),
+ }))
+ })
+ }
+}
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v8 0/2] Introduce Synology Microp driver
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-20 14:24 ` [PATCH v8 2/2] platform: Add initial synology microp driver Markus Probst
@ 2026-04-20 15:55 ` Markus Probst
2 siblings, 0 replies; 19+ messages in thread
From: Markus Probst @ 2026-04-20 15:55 UTC (permalink / raw)
To: Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman
Cc: platform-driver-x86, linux-leds, devicetree, linux-kernel,
rust-for-linux
[-- Attachment #1: Type: text/plain, Size: 929 bytes --]
On Mon, 2026-04-20 at 14:24 +0000, Markus Probst wrote:
>
> To: Markus Probst <markus.probst@posteo.de>
> To: Rob Herring <robh@kernel.org>
> To: Krzysztof Kozlowski <krzk+dt@kernel.org>
> To: Conor Dooley <conor+dt@kernel.org>
> To: Miguel Ojeda <ojeda@kernel.org>
> To: Boqun Feng <boqun@kernel.org>
> To: Gary Guo <gary@garyguo.net>
> To: Björn Roy Baron <bjorn3_gh@protonmail.com>
> To: Benno Lossin <lossin@kernel.org>
> To: Andreas Hindborg <a.hindborg@kernel.org>
> To: Alice Ryhl <aliceryhl@google.com>
> To: Trevor Gross <tmgross@umich.edu>
> To: Danilo Krummrich <dakr@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: rust-for-linux@vger.kernel.org
Thats unintentional. b4 seems to have messed up my cover letter.
Caused by
https://github.com/mricon/b4/commit/bf7beca19f723300d9a911cbf57d2f6aee3d4493
(according to my bisect).
Thanks
- Markus Probst
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 1/2] dt-bindings: embedded-controller: Add synology microp devices
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
0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2026-04-21 7:07 UTC (permalink / raw)
To: Markus Probst
Cc: Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman, platform-driver-x86, linux-leds,
devicetree, linux-kernel, rust-for-linux
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.
> + items:
> + enum:
No, why the list is randomly ordered.
> + - synology,ds923p-microp
> + - synology,ds1522p-microp
And fallback, whichever is that, is not documented alone.
> + - minItems: 4
> + maxItems: 4
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 2/2] platform: Add initial synology microp driver
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 15:33 ` Krzysztof Kozlowski
1 sibling, 1 reply; 19+ messages in thread
From: Ilpo Järvinen @ 2026-04-21 11:59 UTC (permalink / raw)
To: Markus Probst
Cc: Hans de Goede, Bryan O'Donoghue, Lee Jones, Pavel Machek,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, platform-driver-x86, linux-leds, devicetree,
LKML, rust-for-linux
On Mon, 20 Apr 2026, Markus Probst wrote:
> Add a initial synology microp driver, written in Rust.
> The driver targets a microcontroller found in Synology NAS devices. It
> currently only supports controlling of the power led, status led, alert
> led and usb led. Other components such as fan control or handling
> on-device buttons will be added once the required rust abstractions are
> there.
>
> This driver can be used both on arm and x86, thus it goes into the root
> directory of drivers/platform.
>
> Tested successfully on a Synology DS923+.
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> MAINTAINERS | 6 +
> drivers/platform/Kconfig | 2 +
> drivers/platform/Makefile | 1 +
> drivers/platform/synology_microp/Kconfig | 13 +
> drivers/platform/synology_microp/Makefile | 3 +
> drivers/platform/synology_microp/TODO | 7 +
> drivers/platform/synology_microp/command.rs | 54 ++++
> drivers/platform/synology_microp/led.rs | 281 +++++++++++++++++++++
> drivers/platform/synology_microp/model.rs | 49 ++++
> .../platform/synology_microp/synology_microp.rs | 110 ++++++++
> 10 files changed, 526 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c1c686846cdd..49f08290eed0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -25555,6 +25555,12 @@ F: drivers/dma-buf/sync_*
> F: include/linux/sync_file.h
> F: include/uapi/linux/sync_file.h
>
> +SYNOLOGY MICROP DRIVER
> +M: Markus Probst <markus.probst@posteo.de>
You should probably add:
L: platform-driver-x86@vger.kernel.org
Through which tree the patches to this driver are generally expected to be
picked up?
> +S: Maintained
> +F: Documentation/devicetree/bindings/embedded-controller/synology,ds1825p-microp.yaml
> +F: drivers/platform/synology_microp/
> +
> SYNOPSYS ARC ARCHITECTURE
> M: Vineet Gupta <vgupta@kernel.org>
> L: linux-snps-arc@lists.infradead.org
> diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
> index 312788f249c9..996050566a4a 100644
> --- a/drivers/platform/Kconfig
> +++ b/drivers/platform/Kconfig
> @@ -22,3 +22,5 @@ source "drivers/platform/arm64/Kconfig"
> source "drivers/platform/raspberrypi/Kconfig"
>
> source "drivers/platform/wmi/Kconfig"
> +
> +source "drivers/platform/synology_microp/Kconfig"
> diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
> index fa322e7f8716..2381872e9133 100644
> --- a/drivers/platform/Makefile
> +++ b/drivers/platform/Makefile
> @@ -15,3 +15,4 @@ obj-$(CONFIG_SURFACE_PLATFORMS) += surface/
> obj-$(CONFIG_ARM64_PLATFORM_DEVICES) += arm64/
> obj-$(CONFIG_BCM2835_VCHIQ) += raspberrypi/
> obj-$(CONFIG_ACPI_WMI) += wmi/
> +obj-$(CONFIG_SYNOLOGY_MICROP) += synology_microp/
> diff --git a/drivers/platform/synology_microp/Kconfig b/drivers/platform/synology_microp/Kconfig
> new file mode 100644
> index 000000000000..7c4d8f2808f0
> --- /dev/null
> +++ b/drivers/platform/synology_microp/Kconfig
> @@ -0,0 +1,13 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +config SYNOLOGY_MICROP
> + tristate "Synology Microp driver"
> + depends on LEDS_CLASS && LEDS_CLASS_MULTICOLOR
> + depends on RUST_SERIAL_DEV_BUS_ABSTRACTIONS
> + help
> + Enable support for the MCU found in Synology NAS devices.
> +
> + This is needed to properly shutdown and reboot the device, as well as
> + additional functionality like fan and LED control.
> +
> + This driver is work in progress and may not be fully functional.
> diff --git a/drivers/platform/synology_microp/Makefile b/drivers/platform/synology_microp/Makefile
> new file mode 100644
> index 000000000000..63585ccf76e4
> --- /dev/null
> +++ b/drivers/platform/synology_microp/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-y += synology_microp.o
> diff --git a/drivers/platform/synology_microp/TODO b/drivers/platform/synology_microp/TODO
> new file mode 100644
> index 000000000000..1961a33115db
> --- /dev/null
> +++ b/drivers/platform/synology_microp/TODO
> @@ -0,0 +1,7 @@
> +TODO:
> +- add missing components:
> + - handle on-device buttons (Power, Factory reset, "USB Copy")
> + - handle fan failure
> + - beeper
> + - fan speed control
> + - correctly perform device power-off and restart on Synology devices
Is this TODO list really needed within the kernel distribution?
If you planning on add these features (relatively) soon yourself (perhaps
depending on when the rust infra required for these features becomes
available), the list would not be that useful for other developers at all.
> diff --git a/drivers/platform/synology_microp/command.rs b/drivers/platform/synology_microp/command.rs
> new file mode 100644
> index 000000000000..430cb858e1c3
> --- /dev/null
> +++ b/drivers/platform/synology_microp/command.rs
> @@ -0,0 +1,54 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use kernel::{
> + device::Bound,
> + error::Result,
> + serdev, //
> +};
> +
> +use crate::led;
> +
> +#[expect(
> + clippy::enum_variant_names,
> + reason = "future variants will not end with Led"
> +)]
> +pub(crate) enum Command {
> + PowerLed(led::State),
> + StatusLed(led::StatusLedColor, led::State),
> + AlertLed(led::State),
> + UsbLed(led::State),
> + EsataLed(led::State),
> +}
> +
> +impl Command {
> + pub(crate) fn write(self, dev: &serdev::Device<Bound>) -> Result {
> + dev.write_all(
> + match self {
> + Self::PowerLed(led::State::On) => &[0x34],
> + Self::PowerLed(led::State::Blink) => &[0x35],
> + Self::PowerLed(led::State::Off) => &[0x36],
> +
> + Self::StatusLed(_, led::State::Off) => &[0x37],
> + Self::StatusLed(led::StatusLedColor::Green, led::State::On) => &[0x38],
> + Self::StatusLed(led::StatusLedColor::Green, led::State::Blink) => &[0x39],
> + Self::StatusLed(led::StatusLedColor::Orange, led::State::On) => &[0x3A],
> + Self::StatusLed(led::StatusLedColor::Orange, led::State::Blink) => &[0x3B],
> +
> + Self::AlertLed(led::State::On) => &[0x4C, 0x41, 0x31],
> + Self::AlertLed(led::State::Blink) => &[0x4C, 0x41, 0x32],
> + Self::AlertLed(led::State::Off) => &[0x4C, 0x41, 0x33],
> +
> + Self::UsbLed(led::State::On) => &[0x40],
> + Self::UsbLed(led::State::Blink) => &[0x41],
> + Self::UsbLed(led::State::Off) => &[0x42],
> +
> + Self::EsataLed(led::State::On) => &[0x4C, 0x45, 0x31],
> + Self::EsataLed(led::State::Blink) => &[0x4C, 0x45, 0x32],
> + Self::EsataLed(led::State::Off) => &[0x4C, 0x45, 0x33],
> + },
> + serdev::Timeout::Max,
> + )?;
> + dev.wait_until_sent(serdev::Timeout::Max);
> + Ok(())
> + }
> +}
> diff --git a/drivers/platform/synology_microp/led.rs b/drivers/platform/synology_microp/led.rs
> new file mode 100644
> index 000000000000..f89998a7e6b4
> --- /dev/null
> +++ b/drivers/platform/synology_microp/led.rs
> @@ -0,0 +1,281 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use kernel::{
> + device::Bound,
> + devres::{
> + self,
> + Devres, //
> + },
> + led::{
> + self,
> + LedOps,
> + MultiColorSubLed, //
> + },
> + new_mutex,
> + prelude::*,
> + serdev,
> + str::CString,
> + sync::Mutex, //
> +};
> +use pin_init::pin_init_scope;
> +
> +use crate::{
> + command::Command,
> + model::Model, //
> +};
> +
> +#[pin_data]
> +pub(crate) struct Data {
> + #[pin]
> + status: Devres<led::MultiColorDevice<StatusLedHandler>>,
> + power_name: CString,
> + #[pin]
> + power: Devres<led::Device<LedHandler>>,
> +}
> +
> +impl Data {
> + pub(super) fn register<'a>(
> + dev: &'a serdev::Device<Bound>,
> + model: &'a Model,
> + ) -> impl PinInit<Self, Error> + 'a {
> + pin_init_scope(move || {
> + if let Some(color) = model.led_alert {
> + let name = CString::try_from_fmt(fmt!("{}:alarm", color.as_c_str().to_str()?))?;
> + devres::register(
> + dev.as_ref(),
> + led::DeviceBuilder::new().color(color).name(&name).build(
> + dev,
> + try_pin_init!(LedHandler {
> + blink <- new_mutex!(false),
> + command: Command::AlertLed,
> + }),
> + ),
> + GFP_KERNEL,
> + )?;
> + }
> +
> + if model.led_usb_copy {
> + devres::register(
> + dev.as_ref(),
> + led::DeviceBuilder::new()
> + .color(led::Color::Green)
> + .name(c"green:usb")
> + .build(
> + dev,
> + try_pin_init!(LedHandler {
> + blink <- new_mutex!(false),
> + command: Command::UsbLed,
> + }),
> + ),
> + GFP_KERNEL,
> + )?;
> + }
> +
> + if model.led_esata {
> + devres::register(
> + dev.as_ref(),
> + led::DeviceBuilder::new()
> + .color(led::Color::Green)
> + .name(c"green:esata")
> + .build(
> + dev,
> + try_pin_init!(LedHandler {
> + blink <- new_mutex!(false),
> + command: Command::EsataLed,
> + }),
> + ),
> + GFP_KERNEL,
> + )?;
> + }
> +
> + Ok(try_pin_init!(Self {
> + status <- led::DeviceBuilder::new()
> + .color(led::Color::Multi)
> + .name(c"multicolor:status")
> + .build_multicolor(
> + dev,
> + try_pin_init!(StatusLedHandler {
> + blink <- new_mutex!(false),
> + }),
> + StatusLedHandler::SUBLEDS,
> + ),
> + power_name: CString::try_from_fmt(fmt!(
> + "{}:power",
> + model.led_power.as_c_str().to_str()?
> + ))?,
> + power <- led::DeviceBuilder::new()
> + .color(model.led_power)
> + .name(power_name)
> + .build(
> + dev,
> + try_pin_init!(LedHandler {
> + blink <- new_mutex!(true),
> + command: Command::PowerLed,
> + }),
> + ),
> + }))
> + })
> + }
> +}
> +
> +#[derive(Copy, Clone)]
> +pub(crate) enum StatusLedColor {
> + Green,
> + Orange,
> +}
> +
> +#[derive(Copy, Clone)]
> +pub(crate) enum State {
> + On,
> + Blink,
> + Off,
> +}
> +
> +#[pin_data]
> +struct LedHandler {
> + #[pin]
> + blink: Mutex<bool>,
> + command: fn(State) -> Command,
> +}
> +
> +/// Blink delay measured using video recording on DS923+ for Power and Status Led.
> +///
> +/// We assume it is the same for all other leds and models.
> +const BLINK_DELAY: usize = 167;
On C side time related consts are required to include the unit in their
name. Perhaps Rust code should also follow this convention?
--
i.
> +
> +#[vtable]
> +impl LedOps for LedHandler {
> + type Bus = serdev::Device<Bound>;
> + type Mode = led::Normal;
> + const BLOCKING: bool = true;
> + const MAX_BRIGHTNESS: u32 = 1;
> +
> + fn brightness_set(
> + &self,
> + dev: &Self::Bus,
> + _classdev: &led::Device<Self>,
> + brightness: u32,
> + ) -> Result<()> {
> + let mut blink = self.blink.lock();
> + (self.command)(if brightness == 0 {
> + *blink = false;
> + State::Off
> + } else if *blink {
> + State::Blink
> + } else {
> + State::On
> + })
> + .write(dev)?;
> +
> + Ok(())
> + }
> +
> + fn blink_set(
> + &self,
> + dev: &Self::Bus,
> + _classdev: &led::Device<Self>,
> + delay_on: &mut usize,
> + delay_off: &mut usize,
> + ) -> Result<()> {
> + let mut blink = self.blink.lock();
> +
> + (self.command)(if *delay_on == 0 && *delay_off != 0 {
> + State::Off
> + } else if *delay_on != 0 && *delay_off == 0 {
> + State::On
> + } else {
> + *blink = true;
> + *delay_on = BLINK_DELAY;
> + *delay_off = BLINK_DELAY;
> +
> + State::Blink
> + })
> + .write(dev)
> + }
> +}
> +
> +#[pin_data]
> +struct StatusLedHandler {
> + #[pin]
> + blink: Mutex<bool>,
> +}
> +
> +impl StatusLedHandler {
> + const SUBLEDS: &[MultiColorSubLed] = &[
> + MultiColorSubLed::new(led::Color::Green).initial_intensity(1),
> + MultiColorSubLed::new(led::Color::Orange),
> + ];
> +}
> +
> +#[vtable]
> +impl LedOps for StatusLedHandler {
> + type Bus = serdev::Device<Bound>;
> + type Mode = led::MultiColor;
> + const BLOCKING: bool = true;
> + const MAX_BRIGHTNESS: u32 = 1;
> +
> + fn brightness_set(
> + &self,
> + dev: &Self::Bus,
> + classdev: &led::MultiColorDevice<Self>,
> + brightness: u32,
> + ) -> Result<()> {
> + let mut blink = self.blink.lock();
> + if brightness == 0 {
> + *blink = false;
> + }
> +
> + let (color, subled_brightness) = if classdev.subleds()[1].intensity == 0 {
> + (StatusLedColor::Green, classdev.subleds()[0].brightness)
> + } else {
> + (StatusLedColor::Orange, classdev.subleds()[1].brightness)
> + };
> +
> + Command::StatusLed(
> + color,
> + if subled_brightness == 0 {
> + State::Off
> + } else if *blink {
> + State::Blink
> + } else {
> + State::On
> + },
> + )
> + .write(dev)
> + }
> +
> + fn blink_set(
> + &self,
> + dev: &Self::Bus,
> + classdev: &led::MultiColorDevice<Self>,
> + delay_on: &mut usize,
> + delay_off: &mut usize,
> + ) -> Result<()> {
> + let mut blink = self.blink.lock();
> + *blink = true;
> +
> + let (color, subled_intensity) = if classdev.subleds()[1].intensity == 0 {
> + (StatusLedColor::Green, classdev.subleds()[0].intensity)
> + } else {
> + (StatusLedColor::Orange, classdev.subleds()[1].intensity)
> + };
> + Command::StatusLed(
> + color,
> + if *delay_on == 0 && *delay_off != 0 {
> + *blink = false;
> + State::Off
> + } else if subled_intensity == 0 {
> + State::Off
> + } else if *delay_on != 0 && *delay_off == 0 {
> + *blink = false;
> + State::On
> + } else {
> + *delay_on = BLINK_DELAY;
> + *delay_off = BLINK_DELAY;
> +
> + State::Blink
> + },
> + )
> + .write(dev)
> + }
> +}
> diff --git a/drivers/platform/synology_microp/model.rs b/drivers/platform/synology_microp/model.rs
> new file mode 100644
> index 000000000000..715d8840f56b
> --- /dev/null
> +++ b/drivers/platform/synology_microp/model.rs
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use kernel::led::Color;
> +
> +pub(crate) struct Model {
> + pub(crate) led_power: Color,
> + pub(crate) led_alert: Option<Color>,
> + pub(crate) led_usb_copy: bool,
> + pub(crate) led_esata: bool,
> +}
> +
> +impl Model {
> + pub(super) const fn new() -> Self {
> + Self {
> + led_power: Color::Blue,
> + led_alert: None,
> + led_usb_copy: false,
> + led_esata: false,
> + }
> + }
> +
> + pub(super) const fn led_power(self, color: Color) -> Self {
> + Self {
> + led_power: color,
> + ..self
> + }
> + }
> +
> + pub(super) const fn led_alert(self, color: Color) -> Self {
> + Self {
> + led_alert: Some(color),
> + ..self
> + }
> + }
> +
> + pub(super) const fn led_esata(self) -> Self {
> + Self {
> + led_esata: true,
> + ..self
> + }
> + }
> +
> + pub(super) const fn led_usb_copy(self) -> Self {
> + Self {
> + led_usb_copy: true,
> + ..self
> + }
> + }
> +}
> diff --git a/drivers/platform/synology_microp/synology_microp.rs b/drivers/platform/synology_microp/synology_microp.rs
> new file mode 100644
> index 000000000000..1fd4fc658d85
> --- /dev/null
> +++ b/drivers/platform/synology_microp/synology_microp.rs
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Synology Microp driver
> +
> +use kernel::{
> + device,
> + led::Color,
> + of::{
> + DeviceId,
> + IdTable, //
> + },
> + of_device_table,
> + prelude::*,
> + serdev, //
> +};
> +use pin_init::pin_init_scope;
> +
> +use crate::model::Model;
> +
> +pub(crate) mod command;
> +mod led;
> +mod model;
> +
> +kernel::module_serdev_device_driver! {
> + type: SynologyMicropDriver,
> + name: "synology_microp",
> + authors: ["Markus Probst <markus.probst@posteo.de>"],
> + description: "Synology Microp driver",
> + license: "GPL v2",
> +}
> +
> +#[rustfmt::skip]
> +of_device_table!(
> + OF_TABLE,
> + MODULE_OF_TABLE,
> + Model,
> + [
> + // apollolake
> + (DeviceId::new(c"synology,ds918p-microp"), Model::new()),
> +
> + // evansport
> + (DeviceId::new(c"synology,ds214play-microp"), Model::new()),
> +
> + // geminilakenk
> + (DeviceId::new(c"synology,ds225p-microp"), Model::new().led_usb_copy()),
> + (DeviceId::new(c"synology,ds425p-microp"), Model::new()),
> +
> + // pineview
> + (DeviceId::new(c"synology,ds710p-microp"), Model::new().led_esata()),
> + (DeviceId::new(c"synology,ds1010p-microp"), Model::new().led_alert(Color::Orange)),
> + (DeviceId::new(c"synology,ds411p-microp"), Model::new()),
> +
> + // r1000
> + (DeviceId::new(c"synology,ds923p-microp"), Model::new()),
> + (DeviceId::new(c"synology,ds723p-microp"), Model::new()),
> + (DeviceId::new(c"synology,ds1522p-microp"), Model::new()),
> + (DeviceId::new(c"synology,rs422p-microp"), Model::new().led_power(Color::Green)),
> +
> + // r1000nk
> + (DeviceId::new(c"synology,ds725p-microp"), Model::new()),
> +
> + // rtd1296
> + (DeviceId::new(c"synology,ds118-microp"), Model::new()),
> +
> + // rtd1619b
> + (DeviceId::new(c"synology,ds124-microp"), Model::new()),
> + (DeviceId::new(c"synolody,ds223-microp"), Model::new().led_usb_copy()),
> + (DeviceId::new(c"synology,ds223j-microp"), Model::new()),
> +
> + // v1000
> + (DeviceId::new(c"synology,ds1823xsp-microp"), Model::new()),
> + (DeviceId::new(c"synology,rs822p-microp"), Model::new().led_power(Color::Green)),
> + (DeviceId::new(c"synology,rs1221p-microp"), Model::new().led_power(Color::Green)),
> + (DeviceId::new(c"synology,rs1221rpp-microp"), Model::new().led_power(Color::Green)),
> +
> + // v1000nk
> + (DeviceId::new(c"synology,ds925p-microp"), Model::new()),
> + (DeviceId::new(c"synology,ds1525p-microp"), Model::new()),
> + (DeviceId::new(c"synology,ds1825p-microp"), Model::new()),
> + ]
> +);
> +
> +#[pin_data]
> +struct SynologyMicropDriver {
> + #[pin]
> + led: led::Data,
> +}
> +
> +#[vtable]
> +impl serdev::Driver for SynologyMicropDriver {
> + type IdInfo = Model;
> + const OF_ID_TABLE: Option<IdTable<Self::IdInfo>> = Some(&OF_TABLE);
> +
> + fn probe(
> + dev: &serdev::Device<device::Core>,
> + model: Option<&Model>,
> + ) -> impl PinInit<Self, kernel::error::Error> {
> + pin_init_scope(move || {
> + let model = model.ok_or(EINVAL)?;
> +
> + dev.set_baudrate(9600).map_err(|_| EINVAL)?;
> + dev.set_flow_control(false);
> + dev.set_parity(serdev::Parity::None)?;
> +
> + Ok(try_pin_init!(Self {
> + led <- led::Data::register(dev, model),
> + }))
> + })
> + }
> +}
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 2/2] platform: Add initial synology microp driver
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
0 siblings, 2 replies; 19+ messages in thread
From: Markus Probst @ 2026-04-21 14:17 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Hans de Goede, Bryan O'Donoghue, Lee Jones, Pavel Machek,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, platform-driver-x86, linux-leds, devicetree,
LKML, rust-for-linux
[-- Attachment #1: Type: text/plain, Size: 13502 bytes --]
On Tue, 2026-04-21 at 14:59 +0300, Ilpo Järvinen wrote:
> On Mon, 20 Apr 2026, Markus Probst wrote:
>
> > Add a initial synology microp driver, written in Rust.
> > The driver targets a microcontroller found in Synology NAS devices. It
> > currently only supports controlling of the power led, status led, alert
> > led and usb led. Other components such as fan control or handling
> > on-device buttons will be added once the required rust abstractions are
> > there.
> >
> > This driver can be used both on arm and x86, thus it goes into the root
> > directory of drivers/platform.
> >
> > Tested successfully on a Synology DS923+.
> >
> > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > ---
> > MAINTAINERS | 6 +
> > drivers/platform/Kconfig | 2 +
> > drivers/platform/Makefile | 1 +
> > drivers/platform/synology_microp/Kconfig | 13 +
> > drivers/platform/synology_microp/Makefile | 3 +
> > drivers/platform/synology_microp/TODO | 7 +
> > drivers/platform/synology_microp/command.rs | 54 ++++
> > drivers/platform/synology_microp/led.rs | 281 +++++++++++++++++++++
> > drivers/platform/synology_microp/model.rs | 49 ++++
> > .../platform/synology_microp/synology_microp.rs | 110 ++++++++
> > 10 files changed, 526 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index c1c686846cdd..49f08290eed0 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -25555,6 +25555,12 @@ F: drivers/dma-buf/sync_*
> > F: include/linux/sync_file.h
> > F: include/uapi/linux/sync_file.h
> >
> > +SYNOLOGY MICROP DRIVER
> > +M: Markus Probst <markus.probst@posteo.de>
>
> You should probably add:
>
> L: platform-driver-x86@vger.kernel.org
>
> Through which tree the patches to this driver are generally expected to be
> picked up?
I suppose platform-drivers-x86. The driver itself can be used both on
x86 and arm64. Although I also have seen Synology devices with PowerPC
(no device with PowerPC is supported in the driver yet).
>
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/embedded-controller/synology,ds1825p-microp.yaml
> > +F: drivers/platform/synology_microp/
> > +
> > SYNOPSYS ARC ARCHITECTURE
> > M: Vineet Gupta <vgupta@kernel.org>
> > L: linux-snps-arc@lists.infradead.org
> > diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
> > index 312788f249c9..996050566a4a 100644
> > --- a/drivers/platform/Kconfig
> > +++ b/drivers/platform/Kconfig
> > @@ -22,3 +22,5 @@ source "drivers/platform/arm64/Kconfig"
> > source "drivers/platform/raspberrypi/Kconfig"
> >
> > source "drivers/platform/wmi/Kconfig"
> > +
> > +source "drivers/platform/synology_microp/Kconfig"
> > diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
> > index fa322e7f8716..2381872e9133 100644
> > --- a/drivers/platform/Makefile
> > +++ b/drivers/platform/Makefile
> > @@ -15,3 +15,4 @@ obj-$(CONFIG_SURFACE_PLATFORMS) += surface/
> > obj-$(CONFIG_ARM64_PLATFORM_DEVICES) += arm64/
> > obj-$(CONFIG_BCM2835_VCHIQ) += raspberrypi/
> > obj-$(CONFIG_ACPI_WMI) += wmi/
> > +obj-$(CONFIG_SYNOLOGY_MICROP) += synology_microp/
> > diff --git a/drivers/platform/synology_microp/Kconfig b/drivers/platform/synology_microp/Kconfig
> > new file mode 100644
> > index 000000000000..7c4d8f2808f0
> > --- /dev/null
> > +++ b/drivers/platform/synology_microp/Kconfig
> > @@ -0,0 +1,13 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +config SYNOLOGY_MICROP
> > + tristate "Synology Microp driver"
> > + depends on LEDS_CLASS && LEDS_CLASS_MULTICOLOR
> > + depends on RUST_SERIAL_DEV_BUS_ABSTRACTIONS
> > + help
> > + Enable support for the MCU found in Synology NAS devices.
> > +
> > + This is needed to properly shutdown and reboot the device, as well as
> > + additional functionality like fan and LED control.
> > +
> > + This driver is work in progress and may not be fully functional.
> > diff --git a/drivers/platform/synology_microp/Makefile b/drivers/platform/synology_microp/Makefile
> > new file mode 100644
> > index 000000000000..63585ccf76e4
> > --- /dev/null
> > +++ b/drivers/platform/synology_microp/Makefile
> > @@ -0,0 +1,3 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +obj-y += synology_microp.o
> > diff --git a/drivers/platform/synology_microp/TODO b/drivers/platform/synology_microp/TODO
> > new file mode 100644
> > index 000000000000..1961a33115db
> > --- /dev/null
> > +++ b/drivers/platform/synology_microp/TODO
> > @@ -0,0 +1,7 @@
> > +TODO:
> > +- add missing components:
> > + - handle on-device buttons (Power, Factory reset, "USB Copy")
> > + - handle fan failure
> > + - beeper
> > + - fan speed control
> > + - correctly perform device power-off and restart on Synology devices
>
> Is this TODO list really needed within the kernel distribution?
Not really. Although it indicates the current state of the driver.
>
> If you planning on add these features (relatively) soon yourself (perhaps
> depending on when the rust infra required for these features becomes
> available), the list would not be that useful for other developers at all.
Yes. Also I haven't seen anyone work on input, hwmon, reboot/sysoff
rust abstractions yet, so I will likely need to add those as well.
>
> > diff --git a/drivers/platform/synology_microp/command.rs b/drivers/platform/synology_microp/command.rs
> > new file mode 100644
> > index 000000000000..430cb858e1c3
> > --- /dev/null
> > +++ b/drivers/platform/synology_microp/command.rs
> > @@ -0,0 +1,54 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +use kernel::{
> > + device::Bound,
> > + error::Result,
> > + serdev, //
> > +};
> > +
> > +use crate::led;
> > +
> > +#[expect(
> > + clippy::enum_variant_names,
> > + reason = "future variants will not end with Led"
> > +)]
> > +pub(crate) enum Command {
> > + PowerLed(led::State),
> > + StatusLed(led::StatusLedColor, led::State),
> > + AlertLed(led::State),
> > + UsbLed(led::State),
> > + EsataLed(led::State),
> > +}
> > +
> > +impl Command {
> > + pub(crate) fn write(self, dev: &serdev::Device<Bound>) -> Result {
> > + dev.write_all(
> > + match self {
> > + Self::PowerLed(led::State::On) => &[0x34],
> > + Self::PowerLed(led::State::Blink) => &[0x35],
> > + Self::PowerLed(led::State::Off) => &[0x36],
> > +
> > + Self::StatusLed(_, led::State::Off) => &[0x37],
> > + Self::StatusLed(led::StatusLedColor::Green, led::State::On) => &[0x38],
> > + Self::StatusLed(led::StatusLedColor::Green, led::State::Blink) => &[0x39],
> > + Self::StatusLed(led::StatusLedColor::Orange, led::State::On) => &[0x3A],
> > + Self::StatusLed(led::StatusLedColor::Orange, led::State::Blink) => &[0x3B],
> > +
> > + Self::AlertLed(led::State::On) => &[0x4C, 0x41, 0x31],
> > + Self::AlertLed(led::State::Blink) => &[0x4C, 0x41, 0x32],
> > + Self::AlertLed(led::State::Off) => &[0x4C, 0x41, 0x33],
> > +
> > + Self::UsbLed(led::State::On) => &[0x40],
> > + Self::UsbLed(led::State::Blink) => &[0x41],
> > + Self::UsbLed(led::State::Off) => &[0x42],
> > +
> > + Self::EsataLed(led::State::On) => &[0x4C, 0x45, 0x31],
> > + Self::EsataLed(led::State::Blink) => &[0x4C, 0x45, 0x32],
> > + Self::EsataLed(led::State::Off) => &[0x4C, 0x45, 0x33],
> > + },
> > + serdev::Timeout::Max,
> > + )?;
> > + dev.wait_until_sent(serdev::Timeout::Max);
> > + Ok(())
> > + }
> > +}
> > diff --git a/drivers/platform/synology_microp/led.rs b/drivers/platform/synology_microp/led.rs
> > new file mode 100644
> > index 000000000000..f89998a7e6b4
> > --- /dev/null
> > +++ b/drivers/platform/synology_microp/led.rs
> > @@ -0,0 +1,281 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +use kernel::{
> > + device::Bound,
> > + devres::{
> > + self,
> > + Devres, //
> > + },
> > + led::{
> > + self,
> > + LedOps,
> > + MultiColorSubLed, //
> > + },
> > + new_mutex,
> > + prelude::*,
> > + serdev,
> > + str::CString,
> > + sync::Mutex, //
> > +};
> > +use pin_init::pin_init_scope;
> > +
> > +use crate::{
> > + command::Command,
> > + model::Model, //
> > +};
> > +
> > +#[pin_data]
> > +pub(crate) struct Data {
> > + #[pin]
> > + status: Devres<led::MultiColorDevice<StatusLedHandler>>,
> > + power_name: CString,
> > + #[pin]
> > + power: Devres<led::Device<LedHandler>>,
> > +}
> > +
> > +impl Data {
> > + pub(super) fn register<'a>(
> > + dev: &'a serdev::Device<Bound>,
> > + model: &'a Model,
> > + ) -> impl PinInit<Self, Error> + 'a {
> > + pin_init_scope(move || {
> > + if let Some(color) = model.led_alert {
> > + let name = CString::try_from_fmt(fmt!("{}:alarm", color.as_c_str().to_str()?))?;
> > + devres::register(
> > + dev.as_ref(),
> > + led::DeviceBuilder::new().color(color).name(&name).build(
> > + dev,
> > + try_pin_init!(LedHandler {
> > + blink <- new_mutex!(false),
> > + command: Command::AlertLed,
> > + }),
> > + ),
> > + GFP_KERNEL,
> > + )?;
> > + }
> > +
> > + if model.led_usb_copy {
> > + devres::register(
> > + dev.as_ref(),
> > + led::DeviceBuilder::new()
> > + .color(led::Color::Green)
> > + .name(c"green:usb")
> > + .build(
> > + dev,
> > + try_pin_init!(LedHandler {
> > + blink <- new_mutex!(false),
> > + command: Command::UsbLed,
> > + }),
> > + ),
> > + GFP_KERNEL,
> > + )?;
> > + }
> > +
> > + if model.led_esata {
> > + devres::register(
> > + dev.as_ref(),
> > + led::DeviceBuilder::new()
> > + .color(led::Color::Green)
> > + .name(c"green:esata")
> > + .build(
> > + dev,
> > + try_pin_init!(LedHandler {
> > + blink <- new_mutex!(false),
> > + command: Command::EsataLed,
> > + }),
> > + ),
> > + GFP_KERNEL,
> > + )?;
> > + }
> > +
> > + Ok(try_pin_init!(Self {
> > + status <- led::DeviceBuilder::new()
> > + .color(led::Color::Multi)
> > + .name(c"multicolor:status")
> > + .build_multicolor(
> > + dev,
> > + try_pin_init!(StatusLedHandler {
> > + blink <- new_mutex!(false),
> > + }),
> > + StatusLedHandler::SUBLEDS,
> > + ),
> > + power_name: CString::try_from_fmt(fmt!(
> > + "{}:power",
> > + model.led_power.as_c_str().to_str()?
> > + ))?,
> > + power <- led::DeviceBuilder::new()
> > + .color(model.led_power)
> > + .name(power_name)
> > + .build(
> > + dev,
> > + try_pin_init!(LedHandler {
> > + blink <- new_mutex!(true),
> > + command: Command::PowerLed,
> > + }),
> > + ),
> > + }))
> > + })
> > + }
> > +}
> > +
> > +#[derive(Copy, Clone)]
> > +pub(crate) enum StatusLedColor {
> > + Green,
> > + Orange,
> > +}
> > +
> > +#[derive(Copy, Clone)]
> > +pub(crate) enum State {
> > + On,
> > + Blink,
> > + Off,
> > +}
> > +
> > +#[pin_data]
> > +struct LedHandler {
> > + #[pin]
> > + blink: Mutex<bool>,
> > + command: fn(State) -> Command,
> > +}
> > +
> > +/// Blink delay measured using video recording on DS923+ for Power and Status Led.
> > +///
> > +/// We assume it is the same for all other leds and models.
> > +const BLINK_DELAY: usize = 167;
>
> On C side time related consts are required to include the unit in their
> name. Perhaps Rust code should also follow this convention?
How about `const BLINK_DELAY: Msecs` ? The unit would be implied
through the already existing type alias `kernel::time::Msecs` for u32.
Thanks
- Markus Probst
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 1/2] dt-bindings: embedded-controller: Add synology microp devices
2026-04-21 7:07 ` Krzysztof Kozlowski
@ 2026-04-21 14:50 ` Markus Probst
2026-04-21 15:32 ` Krzysztof Kozlowski
0 siblings, 1 reply; 19+ messages in thread
From: Markus Probst @ 2026-04-21 14:50 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman, platform-driver-x86, linux-leds,
devicetree, linux-kernel, rust-for-linux
[-- Attachment #1: Type: text/plain, Size: 3955 bytes --]
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. This is what
I did here.
Other sources suggest, I should add fallbacks that are less specific
about the device:
e. g.
- items:
- enum:
- synology,ds923p-microp
- synology,ds723p-microp
- synology,ds1522p-microp
- synology,rs422p-microp
- const: synology,r1000-microp
- const: synology,x86-microp
- items:
- enum:
- synology,ds225p-microp
- synology,ds425p-microp
- const: synology,geminilakenk-microp
- const: synology,x86-microp
...
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.
>
> > + - 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.
Thanks
- Markus Probst
[1]
https://lore.kernel.org/all/8c8555b3375375dac47a22fad40080fd5b4228a5.camel@posteo.de/
>
> Best regards,
> Krzysztof
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 2/2] platform: Add initial synology microp driver
2026-04-21 14:17 ` Markus Probst
@ 2026-04-21 14:58 ` Miguel Ojeda
2026-04-21 18:10 ` Ilpo Järvinen
1 sibling, 0 replies; 19+ messages in thread
From: Miguel Ojeda @ 2026-04-21 14:58 UTC (permalink / raw)
To: Markus Probst
Cc: Ilpo Järvinen, Hans de Goede, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman, platform-driver-x86, linux-leds,
devicetree, LKML, rust-for-linux
On Tue, Apr 21, 2026 at 4:17 PM Markus Probst <markus.probst@posteo.de> wrote:
>
> How about `const BLINK_DELAY: Msecs` ? The unit would be implied
> through the already existing type alias `kernel::time::Msecs` for u32.
Ideally we would use strong types for things like this, i.e. Rust
newtypes, rather than using type aliases.
e.g. see our `Delta` type.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 1/2] dt-bindings: embedded-controller: Add synology microp devices
2026-04-21 14:50 ` Markus Probst
@ 2026-04-21 15:32 ` Krzysztof Kozlowski
2026-04-21 16:25 ` Markus Probst
0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2026-04-21 15:32 UTC (permalink / raw)
To: Markus Probst
Cc: Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman, platform-driver-x86, linux-leds,
devicetree, linux-kernel, rust-for-linux
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 2/2] platform: Add initial synology microp driver
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 15:33 ` Krzysztof Kozlowski
2026-04-21 16:29 ` Markus Probst
1 sibling, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2026-04-21 15:33 UTC (permalink / raw)
To: Markus Probst, Hans de Goede, Ilpo Järvinen,
Bryan O'Donoghue, Lee Jones, Pavel Machek, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman
Cc: platform-driver-x86, linux-leds, devicetree, linux-kernel,
rust-for-linux
On 20/04/2026 16:24, Markus Probst wrote:
> + // pineview
> + (DeviceId::new(c"synology,ds710p-microp"), Model::new().led_esata()),
> + (DeviceId::new(c"synology,ds1010p-microp"), Model::new().led_alert(Color::Orange)),
> + (DeviceId::new(c"synology,ds411p-microp"), Model::new()),
> +
> + // r1000
> + (DeviceId::new(c"synology,ds923p-microp"), Model::new()),
> + (DeviceId::new(c"synology,ds723p-microp"), Model::new()),
> + (DeviceId::new(c"synology,ds1522p-microp"), Model::new()),
What is this all doing here? Again, what is the fallback and front
compatible? Why do you keep duplicating all this when I asked to REMOVE
the completely unnecessary front compatibles?
So it is not only schema which is wrong, but your driver makes no sense
with it.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 1/2] dt-bindings: embedded-controller: Add synology microp devices
2026-04-21 15:32 ` Krzysztof Kozlowski
@ 2026-04-21 16:25 ` Markus Probst
2026-04-23 9:38 ` Krzysztof Kozlowski
0 siblings, 1 reply; 19+ messages in thread
From: Markus Probst @ 2026-04-21 16:25 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman, platform-driver-x86, linux-leds,
devicetree, linux-kernel, rust-for-linux
[-- 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 --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 2/2] platform: Add initial synology microp driver
2026-04-21 15:33 ` Krzysztof Kozlowski
@ 2026-04-21 16:29 ` Markus Probst
0 siblings, 0 replies; 19+ messages in thread
From: Markus Probst @ 2026-04-21 16:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Hans de Goede, Ilpo Järvinen,
Bryan O'Donoghue, Lee Jones, Pavel Machek, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman
Cc: platform-driver-x86, linux-leds, devicetree, linux-kernel,
rust-for-linux
[-- Attachment #1: Type: text/plain, Size: 1249 bytes --]
On Tue, 2026-04-21 at 17:33 +0200, Krzysztof Kozlowski wrote:
> On 20/04/2026 16:24, Markus Probst wrote:
> > + // pineview
> > + (DeviceId::new(c"synology,ds710p-microp"), Model::new().led_esata()),
> > + (DeviceId::new(c"synology,ds1010p-microp"), Model::new().led_alert(Color::Orange)),
> > + (DeviceId::new(c"synology,ds411p-microp"), Model::new()),
> > +
> > + // r1000
> > + (DeviceId::new(c"synology,ds923p-microp"), Model::new()),
> > + (DeviceId::new(c"synology,ds723p-microp"), Model::new()),
> > + (DeviceId::new(c"synology,ds1522p-microp"), Model::new()),
>
> What is this all doing here? Again, what is the fallback and front
> compatible?
>
See previous comment. There was no separation between them until now. I
will fix that.
> Why do you keep duplicating all this when I asked to REMOVE
> the completely unnecessary front compatibles?
I can't recall you asking that. My apologies if I missed that.
So only keep the fallbacks and only add a front compatible if a
unexpected difference comes up?
Thanks
- Markus Probst
>
> So it is not only schema which is wrong, but your driver makes no sense
> with it.
>
> Best regards,
> Krzysztof
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 2/2] platform: Add initial synology microp driver
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
1 sibling, 1 reply; 19+ messages in thread
From: Ilpo Järvinen @ 2026-04-21 18:10 UTC (permalink / raw)
To: Markus Probst
Cc: Hans de Goede, Bryan O'Donoghue, Lee Jones, Pavel Machek,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, platform-driver-x86, linux-leds, devicetree,
LKML, rust-for-linux
[-- Attachment #1: Type: text/plain, Size: 4741 bytes --]
On Tue, 21 Apr 2026, Markus Probst wrote:
> On Tue, 2026-04-21 at 14:59 +0300, Ilpo Järvinen wrote:
> > On Mon, 20 Apr 2026, Markus Probst wrote:
> >
> > > Add a initial synology microp driver, written in Rust.
> > > The driver targets a microcontroller found in Synology NAS devices. It
> > > currently only supports controlling of the power led, status led, alert
> > > led and usb led. Other components such as fan control or handling
> > > on-device buttons will be added once the required rust abstractions are
> > > there.
> > >
> > > This driver can be used both on arm and x86, thus it goes into the root
> > > directory of drivers/platform.
> > >
> > > Tested successfully on a Synology DS923+.
> > >
> > > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > > ---
> > > MAINTAINERS | 6 +
> > > drivers/platform/Kconfig | 2 +
> > > drivers/platform/Makefile | 1 +
> > > drivers/platform/synology_microp/Kconfig | 13 +
> > > drivers/platform/synology_microp/Makefile | 3 +
> > > drivers/platform/synology_microp/TODO | 7 +
> > > drivers/platform/synology_microp/command.rs | 54 ++++
> > > drivers/platform/synology_microp/led.rs | 281 +++++++++++++++++++++
> > > drivers/platform/synology_microp/model.rs | 49 ++++
> > > .../platform/synology_microp/synology_microp.rs | 110 ++++++++
> > > 10 files changed, 526 insertions(+)
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index c1c686846cdd..49f08290eed0 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -25555,6 +25555,12 @@ F: drivers/dma-buf/sync_*
> > > F: include/linux/sync_file.h
> > > F: include/uapi/linux/sync_file.h
> > >
> > > +SYNOLOGY MICROP DRIVER
> > > +M: Markus Probst <markus.probst@posteo.de>
> >
> > You should probably add:
> >
> > L: platform-driver-x86@vger.kernel.org
> >
> > Through which tree the patches to this driver are generally expected to be
> > picked up?
>
> I suppose platform-drivers-x86.
Okay (with the platform drivers maintainer hat on). Just don't expect me
to have deep Rust knowledge.
> The driver itself can be used both on
> x86 and arm64. Although I also have seen Synology devices with PowerPC
> (no device with PowerPC is supported in the driver yet).
In practice platform drivers scope has already expanded beyond x86 so the
platform-drivers-x86 list naming is just a historic artifact.
> > > +S: Maintained
> > > +F: Documentation/devicetree/bindings/embedded-controller/synology,ds1825p-microp.yaml
> > > +F: drivers/platform/synology_microp/
> > > +
> > > SYNOPSYS ARC ARCHITECTURE
> > > M: Vineet Gupta <vgupta@kernel.org>
> > > L: linux-snps-arc@lists.infradead.org
> > > diff --git a/drivers/platform/synology_microp/TODO b/drivers/platform/synology_microp/TODO
> > > new file mode 100644
> > > index 000000000000..1961a33115db
> > > --- /dev/null
> > > +++ b/drivers/platform/synology_microp/TODO
> > > @@ -0,0 +1,7 @@
> > > +TODO:
> > > +- add missing components:
> > > + - handle on-device buttons (Power, Factory reset, "USB Copy")
> > > + - handle fan failure
> > > + - beeper
> > > + - fan speed control
> > > + - correctly perform device power-off and restart on Synology devices
> >
> > Is this TODO list really needed within the kernel distribution?
>
> Not really. Although it indicates the current state of the driver.
>
> > If you planning on add these features (relatively) soon yourself (perhaps
> > depending on when the rust infra required for these features becomes
> > available), the list would not be that useful for other developers at all.
>
> Yes. Also I haven't seen anyone work on input, hwmon, reboot/sysoff
> rust abstractions yet, so I will likely need to add those as well.
Lets not include the TODO file then.
> > > +/// Blink delay measured using video recording on DS923+ for Power and Status Led.
> > > +///
> > > +/// We assume it is the same for all other leds and models.
> > > +const BLINK_DELAY: usize = 167;
> >
> > On C side time related consts are required to include the unit in their
> > name. Perhaps Rust code should also follow this convention?
>
> How about `const BLINK_DELAY: Msecs` ? The unit would be implied
> through the already existing type alias `kernel::time::Msecs` for u32.
I don't have opinion on this with my limited Rust knowledge (it just
stuck to my eye how non-specific that original one looked). If Rust
can do things even better as Miguel seems to imply, please look at those
directions.
--
i.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 2/2] platform: Add initial synology microp driver
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
0 siblings, 2 replies; 19+ messages in thread
From: Markus Probst @ 2026-04-21 18:20 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Hans de Goede, Bryan O'Donoghue, Lee Jones, Pavel Machek,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, platform-driver-x86, linux-leds, devicetree,
LKML, rust-for-linux
[-- Attachment #1: Type: text/plain, Size: 5501 bytes --]
On Tue, 2026-04-21 at 21:10 +0300, Ilpo Järvinen wrote:
> On Tue, 21 Apr 2026, Markus Probst wrote:
>
> > On Tue, 2026-04-21 at 14:59 +0300, Ilpo Järvinen wrote:
> > > On Mon, 20 Apr 2026, Markus Probst wrote:
> > >
> > > > Add a initial synology microp driver, written in Rust.
> > > > The driver targets a microcontroller found in Synology NAS devices. It
> > > > currently only supports controlling of the power led, status led, alert
> > > > led and usb led. Other components such as fan control or handling
> > > > on-device buttons will be added once the required rust abstractions are
> > > > there.
> > > >
> > > > This driver can be used both on arm and x86, thus it goes into the root
> > > > directory of drivers/platform.
> > > >
> > > > Tested successfully on a Synology DS923+.
> > > >
> > > > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > > > ---
> > > > MAINTAINERS | 6 +
> > > > drivers/platform/Kconfig | 2 +
> > > > drivers/platform/Makefile | 1 +
> > > > drivers/platform/synology_microp/Kconfig | 13 +
> > > > drivers/platform/synology_microp/Makefile | 3 +
> > > > drivers/platform/synology_microp/TODO | 7 +
> > > > drivers/platform/synology_microp/command.rs | 54 ++++
> > > > drivers/platform/synology_microp/led.rs | 281 +++++++++++++++++++++
> > > > drivers/platform/synology_microp/model.rs | 49 ++++
> > > > .../platform/synology_microp/synology_microp.rs | 110 ++++++++
> > > > 10 files changed, 526 insertions(+)
> > > >
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index c1c686846cdd..49f08290eed0 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -25555,6 +25555,12 @@ F: drivers/dma-buf/sync_*
> > > > F: include/linux/sync_file.h
> > > > F: include/uapi/linux/sync_file.h
> > > >
> > > > +SYNOLOGY MICROP DRIVER
> > > > +M: Markus Probst <markus.probst@posteo.de>
> > >
> > > You should probably add:
> > >
> > > L: platform-driver-x86@vger.kernel.org
> > >
> > > Through which tree the patches to this driver are generally expected to be
> > > picked up?
> >
> > I suppose platform-drivers-x86.
>
> Okay (with the platform drivers maintainer hat on). Just don't expect me
> to have deep Rust knowledge.
>
> > The driver itself can be used both on
> > x86 and arm64. Although I also have seen Synology devices with PowerPC
> > (no device with PowerPC is supported in the driver yet).
>
> In practice platform drivers scope has already expanded beyond x86 so the
> platform-drivers-x86 list naming is just a historic artifact.
Does this also include the drivers/platform/x86 folder?
Because of the multiple architectures, I put it into the root, i. e.
drivers/platform/synology_microp/
Is this fine or should I move it into drivers/platform/x86 ?
>
> > > > +S: Maintained
> > > > +F: Documentation/devicetree/bindings/embedded-controller/synology,ds1825p-microp.yaml
> > > > +F: drivers/platform/synology_microp/
> > > > +
> > > > SYNOPSYS ARC ARCHITECTURE
> > > > M: Vineet Gupta <vgupta@kernel.org>
> > > > L: linux-snps-arc@lists.infradead.org
>
> > > > diff --git a/drivers/platform/synology_microp/TODO b/drivers/platform/synology_microp/TODO
> > > > new file mode 100644
> > > > index 000000000000..1961a33115db
> > > > --- /dev/null
> > > > +++ b/drivers/platform/synology_microp/TODO
> > > > @@ -0,0 +1,7 @@
> > > > +TODO:
> > > > +- add missing components:
> > > > + - handle on-device buttons (Power, Factory reset, "USB Copy")
> > > > + - handle fan failure
> > > > + - beeper
> > > > + - fan speed control
> > > > + - correctly perform device power-off and restart on Synology devices
> > >
> > > Is this TODO list really needed within the kernel distribution?
> >
> > Not really. Although it indicates the current state of the driver.
> >
> > > If you planning on add these features (relatively) soon yourself (perhaps
> > > depending on when the rust infra required for these features becomes
> > > available), the list would not be that useful for other developers at all.
> >
> > Yes. Also I haven't seen anyone work on input, hwmon, reboot/sysoff
> > rust abstractions yet, so I will likely need to add those as well.
>
> Lets not include the TODO file then.
>
> > > > +/// Blink delay measured using video recording on DS923+ for Power and Status Led.
> > > > +///
> > > > +/// We assume it is the same for all other leds and models.
> > > > +const BLINK_DELAY: usize = 167;
> > >
> > > On C side time related consts are required to include the unit in their
> > > name. Perhaps Rust code should also follow this convention?
> >
> > How about `const BLINK_DELAY: Msecs` ? The unit would be implied
> > through the already existing type alias `kernel::time::Msecs` for u32.
>
> I don't have opinion on this with my limited Rust knowledge (it just
> stuck to my eye how non-specific that original one looked). If Rust
> can do things even better as Miguel seems to imply, please look at those
> directions.
Delta stores it in nano seconds, so it will require an additional
`.as_millis()` call on use. I assume rust will optimize that out, so it
will be fine. I will use the `Delta` type like Miguel suggested in the
next revision.
Thanks
- Markus Probst
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 2/2] platform: Add initial synology microp driver
2026-04-21 18:20 ` Markus Probst
@ 2026-04-21 18:36 ` Ilpo Järvinen
2026-04-21 18:46 ` Miguel Ojeda
1 sibling, 0 replies; 19+ messages in thread
From: Ilpo Järvinen @ 2026-04-21 18:36 UTC (permalink / raw)
To: Markus Probst
Cc: Hans de Goede, Bryan O'Donoghue, Lee Jones, Pavel Machek,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, platform-driver-x86, linux-leds, devicetree,
LKML, rust-for-linux
[-- Attachment #1: Type: text/plain, Size: 3692 bytes --]
On Tue, 21 Apr 2026, Markus Probst wrote:
> On Tue, 2026-04-21 at 21:10 +0300, Ilpo Järvinen wrote:
> > On Tue, 21 Apr 2026, Markus Probst wrote:
> >
> > > On Tue, 2026-04-21 at 14:59 +0300, Ilpo Järvinen wrote:
> > > > On Mon, 20 Apr 2026, Markus Probst wrote:
> > > >
> > > > > Add a initial synology microp driver, written in Rust.
> > > > > The driver targets a microcontroller found in Synology NAS devices. It
> > > > > currently only supports controlling of the power led, status led, alert
> > > > > led and usb led. Other components such as fan control or handling
> > > > > on-device buttons will be added once the required rust abstractions are
> > > > > there.
> > > > >
> > > > > This driver can be used both on arm and x86, thus it goes into the root
> > > > > directory of drivers/platform.
> > > > >
> > > > > Tested successfully on a Synology DS923+.
> > > > >
> > > > > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > > > > ---
> > > > > MAINTAINERS | 6 +
> > > > > drivers/platform/Kconfig | 2 +
> > > > > drivers/platform/Makefile | 1 +
> > > > > drivers/platform/synology_microp/Kconfig | 13 +
> > > > > drivers/platform/synology_microp/Makefile | 3 +
> > > > > drivers/platform/synology_microp/TODO | 7 +
> > > > > drivers/platform/synology_microp/command.rs | 54 ++++
> > > > > drivers/platform/synology_microp/led.rs | 281 +++++++++++++++++++++
> > > > > drivers/platform/synology_microp/model.rs | 49 ++++
> > > > > .../platform/synology_microp/synology_microp.rs | 110 ++++++++
> > > > > 10 files changed, 526 insertions(+)
> > > > >
> > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > index c1c686846cdd..49f08290eed0 100644
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -25555,6 +25555,12 @@ F: drivers/dma-buf/sync_*
> > > > > F: include/linux/sync_file.h
> > > > > F: include/uapi/linux/sync_file.h
> > > > >
> > > > > +SYNOLOGY MICROP DRIVER
> > > > > +M: Markus Probst <markus.probst@posteo.de>
> > > >
> > > > You should probably add:
> > > >
> > > > L: platform-driver-x86@vger.kernel.org
> > > >
> > > > Through which tree the patches to this driver are generally expected to be
> > > > picked up?
> > >
> > > I suppose platform-drivers-x86.
> >
> > Okay (with the platform drivers maintainer hat on). Just don't expect me
> > to have deep Rust knowledge.
> >
> > > The driver itself can be used both on
> > > x86 and arm64. Although I also have seen Synology devices with PowerPC
> > > (no device with PowerPC is supported in the driver yet).
> >
> > In practice platform drivers scope has already expanded beyond x86 so the
> > platform-drivers-x86 list naming is just a historic artifact.
>
> Does this also include the drivers/platform/x86 folder?
>
> Because of the multiple architectures, I put it into the root, i. e.
> drivers/platform/synology_microp/
>
> Is this fine or should I move it into drivers/platform/x86 ?
No. The current place you have it is fine with me.
There's actually a small number of files that have migrated away from x86/
when they've become useful for non-x86 systems.
> > > > > +S: Maintained
> > > > > +F: Documentation/devicetree/bindings/embedded-controller/synology,ds1825p-microp.yaml
> > > > > +F: drivers/platform/synology_microp/
> > > > > +
> > > > > SYNOPSYS ARC ARCHITECTURE
> > > > > M: Vineet Gupta <vgupta@kernel.org>
> > > > > L: linux-snps-arc@lists.infradead.org
--
i.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 2/2] platform: Add initial synology microp driver
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
1 sibling, 1 reply; 19+ messages in thread
From: Miguel Ojeda @ 2026-04-21 18:46 UTC (permalink / raw)
To: Markus Probst, Andreas Hindborg, Boqun Feng, FUJITA Tomonori,
Frederic Weisbecker, Lyude Paul, Thomas Gleixner,
Anna-Maria Behnsen, John Stultz, Stephen Boyd
Cc: Ilpo Järvinen, Hans de Goede, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, platform-driver-x86, linux-leds, devicetree,
LKML, rust-for-linux
On Tue, Apr 21, 2026 at 8:21 PM Markus Probst <markus.probst@posteo.de> wrote:
>
> Delta stores it in nano seconds, so it will require an additional
> `.as_millis()` call on use. I assume rust will optimize that out, so it
> will be fine. I will use the `Delta` type like Miguel suggested in the
> next revision.
I think it should (at least in the 64-bit case -- we do have a
`bindings::` C call in the 32-bit case, so likely not in that case),
but please double-check the codegen.
In any case, my suggestion wasn't necessarily about using `Delta`,
which is definitely an option to consider, but rather more generally
about using newtypes, e.g. it may be that we want to have a few simple
time unit types (probably with support for `const`) for cases like
these if people are going to use primitives everywhere to define their
`const`s -- Cc'ing the timekeeping Rust folks.
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 2/2] platform: Add initial synology microp driver
2026-04-21 18:46 ` Miguel Ojeda
@ 2026-04-22 13:48 ` FUJITA Tomonori
0 siblings, 0 replies; 19+ messages in thread
From: FUJITA Tomonori @ 2026-04-22 13:48 UTC (permalink / raw)
To: miguel.ojeda.sandonis
Cc: markus.probst, a.hindborg, boqun, fujita.tomonori, frederic,
lyude, tglx, anna-maria, jstultz, sboyd, ilpo.jarvinen, hansg,
bryan.odonoghue, lee, pavel, ojeda, gary, bjorn3_gh, lossin,
aliceryhl, tmgross, dakr, robh, krzk+dt, conor+dt, gregkh,
platform-driver-x86, linux-leds, devicetree, linux-kernel,
rust-for-linux
On Tue, 21 Apr 2026 20:46:59 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> On Tue, Apr 21, 2026 at 8:21 PM Markus Probst <markus.probst@posteo.de> wrote:
>>
>> Delta stores it in nano seconds, so it will require an additional
>> `.as_millis()` call on use. I assume rust will optimize that out, so it
>> will be fine. I will use the `Delta` type like Miguel suggested in the
>> next revision.
>
> I think it should (at least in the 64-bit case -- we do have a
> `bindings::` C call in the 32-bit case, so likely not in that case),
> but please double-check the codegen.
>
> In any case, my suggestion wasn't necessarily about using `Delta`,
> which is definitely an option to consider, but rather more generally
> about using newtypes, e.g. it may be that we want to have a few simple
> time unit types (probably with support for `const`) for cases like
> these if people are going to use primitives everywhere to define their
> `const`s -- Cc'ing the timekeeping Rust folks.
Currently, Msecs is a type alias, so it provides no type safety, and
it has no users in the kernel. I think we should remove it.
I think that Delta works fine for BLINK_DELAY. On 32-bit there is a
small overhead in as_millis(), but I don't think that will be a
problem significant enough to justify introducing new types.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 1/2] dt-bindings: embedded-controller: Add synology microp devices
2026-04-21 16:25 ` Markus Probst
@ 2026-04-23 9:38 ` Krzysztof Kozlowski
0 siblings, 0 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2026-04-23 9:38 UTC (permalink / raw)
To: Markus Probst
Cc: Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman, platform-driver-x86, linux-leds,
devicetree, linux-kernel, rust-for-linux
On 21/04/2026 18:25, Markus Probst wrote:
>>
>> 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
Only one device is the fallback for given groyp, not each of them.
There is no such syntax in the kernel, nowhere. Please do not invent own
stuff.
> .
>
> 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.
Yep. It is also explained why in writing-bindings - list is STRICTLY
ordered. Not flexible. And example reflects that. BTW, other bindings as
well, so recent EC patches is good way to learn:
(although I understand that in-tree patches do not cover your case)
https://lore.kernel.org/all/20260327-add-driver-for-ec-v7-1-7684c915e42c@oss.qualcomm.com/
from:
https://lore.kernel.org/all/?q=dfn%3Abindings%2Fembedded-controller%2F
>>
>>
>>
>>>
>>>
>>> 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
You did not define ds923p as fallback. It is used as front compatible as
well. Fallback cannot be used in some contexts at front:
+ items:
+ enum:
+ - synology,ds923p-microp
+ - synology,ds1522p-microp
> 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
Yes
>
> 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?
You can open your v1 or other versions. They were quite clear about
fallback: it did not matter for you which device is used as fallback at
v1, because you treated each device as the same programming model. I
assume you treated them the same, because you did not know the
differences (which is fine), so it should not matter that much now.
If unsure, choose oldest device, less features or eventually smallest
number.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2026-04-23 9:38 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox