[PATCH v4 0/2] Introduce Synology Microp driver

[PATCH v4 0/2] Introduce Synology Microp driver

[Markus Probst via B4 Relay]

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/20260207-rust_leds-v12-0-fdb518417b75@posteo.de/
[2] https://lore.kernel.org/rust-for-linux/20260313-rust_serdev-v3-0-c9a3af214f7f@posteo.de/

Signed-off-by: Markus Probst <markus.probst@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

---
Markus Probst (2):
      dt-bindings: embedded-controller: Add synology,microp device
      platform: Add initial synology microp driver

 .../embedded-controller/synology,microp.yaml       |  52 +++++
 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        |  50 ++++
 drivers/platform/synology_microp/led.rs            | 254 +++++++++++++++++++++
 .../platform/synology_microp/synology_microp.rs    |  58 +++++
 10 files changed, 446 insertions(+)
---
base-commit: 34cb4f916af10153c87fabaf6c34e4cafa170427
change-id: 20260306-synology_microp_initial-0f7dac7b7496
prerequisite-change-id: 20251217-rust_serdev-ee5481e9085c:v3
prerequisite-patch-id: 52b17274481cc770c257d8f95335293eca32a2c5
prerequisite-patch-id: eec47e5051640d08bcd34a9670b98804449cad52
prerequisite-patch-id: f24b68c71c3f69371e8ac0251efca0a023b31cc4
prerequisite-patch-id: 3dfc1f7e5ecd3e0dd65d676aeb16f55260847b25
prerequisite-change-id: 20251114-rust_leds-a959f7c2f7f9:v12
prerequisite-patch-id: 42c445ef6981e3a3740dbaaf307f4b810042e46f
prerequisite-patch-id: 90c7b200cca722a592353885e21af069101c4e09
prerequisite-patch-id: c664a52faa3d47000d252eb7603c9c08382e868a

[PATCH v4 1/2] dt-bindings: embedded-controller: Add synology,microp device

[Markus Probst via B4 Relay]

From: Markus Probst <markus.probst@posteo.de>

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.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 .../embedded-controller/synology,microp.yaml       | 52 ++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/Documentation/devicetree/bindings/embedded-controller/synology,microp.yaml b/Documentation/devicetree/bindings/embedded-controller/synology,microp.yaml
new file mode 100644
index 000000000000..3068da6f2a6a
--- /dev/null
+++ b/Documentation/devicetree/bindings/embedded-controller/synology,microp.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/embedded-controller/synology,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:
+    const: synology,microp
+
+patternProperties:
+  "^(power|status|alert|usb)-led$":
+    $ref: /schemas/leds/common.yaml
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - power-led
+  - status-led
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/leds/common.h>
+
+    embedded-controller {
+      compatible = "synology,microp";
+
+      power-led {
+        color = <LED_COLOR_ID_BLUE>;
+        function = LED_FUNCTION_POWER;
+      };
+
+      status-led {
+        color = <LED_COLOR_ID_MULTI>;
+        function = LED_FUNCTION_STATUS;
+      };
+    };

-- 
2.52.0

[PATCH v4 2/2] platform: Add initial synology microp driver

[Markus Probst via B4 Relay]

From: Markus Probst <markus.probst@posteo.de>

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        |  50 ++++
 drivers/platform/synology_microp/led.rs            | 254 +++++++++++++++++++++
 .../platform/synology_microp/synology_microp.rs    |  58 +++++
 9 files changed, 394 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bed1fac09d9f..7ea506ba3fe4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25542,6 +25542,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,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..0c145a5b7174
--- /dev/null
+++ b/drivers/platform/synology_microp/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+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..92deaf5ce3b2
--- /dev/null
+++ b/drivers/platform/synology_microp/command.rs
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use kernel::{
+    device::Bound,
+    error::Result,
+    serdev, //
+};
+
+use crate::led;
+
+#[derive(Copy, Clone)]
+#[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),
+}
+
+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],
+            },
+            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..239d063bc0e9
--- /dev/null
+++ b/drivers/platform/synology_microp/led.rs
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use kernel::{
+    device::{property::FwNode, Bound},
+    devres::{self, Devres},
+    led::{
+        self,
+        LedOps,
+        MultiColorSubLed, //
+    },
+    new_mutex,
+    prelude::*,
+    serdev,
+    sync::Mutex, //
+};
+use pin_init::pin_init_scope;
+
+use crate::command::Command;
+
+#[pin_data]
+pub(crate) struct Data {
+    #[pin]
+    status: Devres<led::MultiColorDevice<StatusLedHandler>>,
+    #[pin]
+    power: Devres<led::Device<LedHandler>>,
+}
+
+impl Data {
+    pub(super) fn register<'a>(
+        dev: &'a serdev::Device<Bound>,
+        fwnode: &'a FwNode,
+    ) -> impl PinInit<Self, Error> + 'a {
+        pin_init_scope(move || {
+            if let Some(alert_fwnode) = fwnode.get_child_by_name(c"alert-led") {
+                devres::register(
+                    dev.as_ref(),
+                    led::DeviceBuilder::new()
+                        .fwnode(Some(alert_fwnode))
+                        .devicename(c"synology-microp/alert-led")
+                        .color(led::Color::Orange)
+                        .build(
+                            dev,
+                            try_pin_init!(LedHandler {
+                                blink <- new_mutex!(false),
+                                command: Command::AlertLed,
+                            }),
+                        ),
+                    GFP_KERNEL,
+                )?;
+            }
+
+            if let Some(alert_fwnode) = fwnode.get_child_by_name(c"usb-led") {
+                devres::register(
+                    dev.as_ref(),
+                    led::DeviceBuilder::new()
+                        .fwnode(Some(alert_fwnode))
+                        .devicename(c"synology-microp/usb-led")
+                        .color(led::Color::Green)
+                        .build(
+                            dev,
+                            try_pin_init!(LedHandler {
+                                blink <- new_mutex!(false),
+                                command: Command::UsbLed,
+                            }),
+                        ),
+                    GFP_KERNEL,
+                )?;
+            }
+
+            Ok(try_pin_init!(Self {
+                status <- led::DeviceBuilder::new()
+                    .fwnode(Some(fwnode.get_child_by_name(c"status-led").ok_or(EINVAL)?))
+                    .devicename(c"synology-microp/status-led")
+                    .color(led::Color::Multi)
+                    .build_multicolor(
+                        dev,
+                        try_pin_init!(StatusLedHandler {
+                            blink <- new_mutex!(false),
+                        }),
+                        StatusLedHandler::SUBLEDS,
+                    ),
+                power <- led::DeviceBuilder::new()
+                    .fwnode(Some(fwnode.get_child_by_name(c"power-led").ok_or(EINVAL)?))
+                    .devicename(c"synology-microp/power-led")
+                    .color(led::Color::Blue)
+                    .default_trigger(c"timer")
+                    .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,
+}
+
+#[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 = 167;
+            *delay_off = 167;
+
+            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 = 167;
+                *delay_off = 167;
+
+                State::Blink
+            },
+        )
+        .write(dev)
+    }
+}
diff --git a/drivers/platform/synology_microp/synology_microp.rs b/drivers/platform/synology_microp/synology_microp.rs
new file mode 100644
index 000000000000..4e3162aa5375
--- /dev/null
+++ b/drivers/platform/synology_microp/synology_microp.rs
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Synology Microp driver
+
+use kernel::{
+    device,
+    of,
+    prelude::*,
+    serdev, //
+};
+use pin_init::pin_init_scope;
+
+pub(crate) mod command;
+mod led;
+
+kernel::module_serdev_device_driver! {
+    type: SynologyMicropDriver,
+    name: "synology_microp",
+    authors: ["Markus Probst <markus.probst@posteo.de>"],
+    description: "Synology Microp driver",
+    license: "GPL v2",
+}
+
+#[pin_data]
+struct SynologyMicropDriver {
+    #[pin]
+    led: led::Data,
+}
+
+kernel::of_device_table!(
+    OF_TABLE,
+    MODULE_OF_TABLE,
+    <SynologyMicropDriver as serdev::Driver>::IdInfo,
+    [(of::DeviceId::new(c"synology,microp"), ()),]
+);
+
+#[vtable]
+impl serdev::Driver for SynologyMicropDriver {
+    type IdInfo = ();
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+
+    fn probe(
+        dev: &serdev::Device<device::Core>,
+        _id_info: Option<&Self::IdInfo>,
+    ) -> impl PinInit<Self, kernel::error::Error> {
+        pin_init_scope(move || {
+            let fwnode = dev.as_ref().fwnode().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, fwnode),
+            }))
+        })
+    }
+}

-- 
2.52.0

Re: [PATCH v4 1/2] dt-bindings: embedded-controller: Add synology,microp device

[Krzysztof Kozlowski]

On Fri, Mar 20, 2026 at 11:09:53PM +0100, Markus Probst wrote:
> +
> +examples:
> +  - |
> +    #include <dt-bindings/leds/common.h>
> +
> +    embedded-controller {
> +      compatible = "synology,microp";
> +
> +      power-led {
> +        color = <LED_COLOR_ID_BLUE>;
> +        function = LED_FUNCTION_POWER;
> +      };
> +
> +      status-led {
> +        color = <LED_COLOR_ID_MULTI>;
> +        function = LED_FUNCTION_STATUS;
> +      };

Where are other leds? Binding mentions 4. Does that mean that they
differ on each device?  The EC is tied to specific model, so that would
be surprising. And if they do not differ, what is exactly the point of
describing the LEDs in DT?

> +    };
> 
> -- 
> 2.52.0
> 

Re: [PATCH v4 1/2] dt-bindings: embedded-controller: Add synology,microp device

[Markus Probst]

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

On Sat, 2026-03-21 at 11:21 +0100, Krzysztof Kozlowski wrote:
> On Fri, Mar 20, 2026 at 11:09:53PM +0100, Markus Probst wrote:
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/leds/common.h>
> > +
> > +    embedded-controller {
> > +      compatible = "synology,microp";
> > +
> > +      power-led {
> > +        color = <LED_COLOR_ID_BLUE>;
> > +        function = LED_FUNCTION_POWER;
> > +      };
> > +
> > +      status-led {
> > +        color = <LED_COLOR_ID_MULTI>;
> > +        function = LED_FUNCTION_STATUS;
> > +      };
> 
> Where are other leds? Binding mentions 4.
> 
Status and Power leds exist on every Synology NAS model I am aware of.
But there are models which have additionally a usb or alert led. The
device nodes for those leds should only be present, if they exist
physically on the device.

> Does that mean that they
> differ on each device?  The EC is tied to specific model, so that would
> be surprising. And if they do not differ, what is exactly the point of
> describing the LEDs in DT?
The color of the leds is different on some models.

I suppose I should add this information to the description.

Thanks
- Markus Probst

> 
> > +    };
> > 
> > -- 
> > 2.52.0
> > 

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

Re: [PATCH v4 1/2] dt-bindings: embedded-controller: Add synology,microp device

[Krzysztof Kozlowski]

On 21/03/2026 13:17, Markus Probst wrote:
> On Sat, 2026-03-21 at 11:21 +0100, Krzysztof Kozlowski wrote:
>> On Fri, Mar 20, 2026 at 11:09:53PM +0100, Markus Probst wrote:
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/leds/common.h>
>>> +
>>> +    embedded-controller {
>>> +      compatible = "synology,microp";
>>> +
>>> +      power-led {
>>> +        color = <LED_COLOR_ID_BLUE>;
>>> +        function = LED_FUNCTION_POWER;
>>> +      };
>>> +
>>> +      status-led {
>>> +        color = <LED_COLOR_ID_MULTI>;
>>> +        function = LED_FUNCTION_STATUS;
>>> +      };
>>
>> Where are other leds? Binding mentions 4.
>>
> Status and Power leds exist on every Synology NAS model I am aware of.
> But there are models which have additionally a usb or alert led. The
> device nodes for those leds should only be present, if they exist
> physically on the device.

Then help me to understand - are these different models?

EC is not a generic purpose component and is tightly coupled with the
actual board it is being present on. Unless exactly same board is used
in different models (unlikely) then the compatible defines the LEDs and
they are not needed in DT.

I should have brought this earlier, so apologies for that.

> 
>> Does that mean that they
>> differ on each device?  The EC is tied to specific model, so that would
>> be surprising. And if they do not differ, what is exactly the point of
>> describing the LEDs in DT?
> The color of the leds is different on some models.
> 
> I suppose I should add this information to the description.
> 


Best regards,
Krzysztof

Re: [PATCH v4 1/2] dt-bindings: embedded-controller: Add synology,microp device

[Markus Probst]

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

On Sat, 2026-03-21 at 13:32 +0100, Krzysztof Kozlowski wrote:
> On 21/03/2026 13:17, Markus Probst wrote:
> > On Sat, 2026-03-21 at 11:21 +0100, Krzysztof Kozlowski wrote:
> > > On Fri, Mar 20, 2026 at 11:09:53PM +0100, Markus Probst wrote:
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    #include <dt-bindings/leds/common.h>
> > > > +
> > > > +    embedded-controller {
> > > > +      compatible = "synology,microp";
> > > > +
> > > > +      power-led {
> > > > +        color = <LED_COLOR_ID_BLUE>;
> > > > +        function = LED_FUNCTION_POWER;
> > > > +      };
> > > > +
> > > > +      status-led {
> > > > +        color = <LED_COLOR_ID_MULTI>;
> > > > +        function = LED_FUNCTION_STATUS;
> > > > +      };
> > > 
> > > Where are other leds? Binding mentions 4.
> > > 
> > Status and Power leds exist on every Synology NAS model I am aware of.
> > But there are models which have additionally a usb or alert led. The
> > device nodes for those leds should only be present, if they exist
> > physically on the device.
> 
> Then help me to understand - are these different models?
Yes, even with different CPU architectures.
How much the "microp" device differs is not clear, but the
communication protocol is the same.
> 
> EC is not a generic purpose component and is tightly coupled with the
> actual board it is being present on. Unless exactly same board is used
> in different models (unlikely) then the compatible defines the LEDs and
> they are not needed in DT.
So for instance "synology,ds923p-microp", "synology,ds723p-microp" etc.
?

I can do that, but that would be many.
Having it generic seems more flexible.

> 
> I should have brought this earlier, so apologies for that.
> 
Thanks
- Markus Probst

> > 
> > > Does that mean that they
> > > differ on each device?  The EC is tied to specific model, so that would
> > > be surprising. And if they do not differ, what is exactly the point of
> > > describing the LEDs in DT?
> > The color of the leds is different on some models.
> > 
> > I suppose I should add this information to the description.
> > 
> 
> 
> Best regards,
> Krzysztof

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

Re: [PATCH v4 1/2] dt-bindings: embedded-controller: Add synology,microp device

[Rob Herring]

On Sat, Mar 21, 2026 at 01:02:22PM +0000, Markus Probst wrote:
> On Sat, 2026-03-21 at 13:32 +0100, Krzysztof Kozlowski wrote:
> > On 21/03/2026 13:17, Markus Probst wrote:
> > > On Sat, 2026-03-21 at 11:21 +0100, Krzysztof Kozlowski wrote:
> > > > On Fri, Mar 20, 2026 at 11:09:53PM +0100, Markus Probst wrote:
> > > > > +
> > > > > +examples:
> > > > > +  - |
> > > > > +    #include <dt-bindings/leds/common.h>
> > > > > +
> > > > > +    embedded-controller {
> > > > > +      compatible = "synology,microp";
> > > > > +
> > > > > +      power-led {
> > > > > +        color = <LED_COLOR_ID_BLUE>;
> > > > > +        function = LED_FUNCTION_POWER;
> > > > > +      };
> > > > > +
> > > > > +      status-led {
> > > > > +        color = <LED_COLOR_ID_MULTI>;
> > > > > +        function = LED_FUNCTION_STATUS;
> > > > > +      };
> > > > 
> > > > Where are other leds? Binding mentions 4.
> > > > 
> > > Status and Power leds exist on every Synology NAS model I am aware of.
> > > But there are models which have additionally a usb or alert led. The
> > > device nodes for those leds should only be present, if they exist
> > > physically on the device.
> > 
> > Then help me to understand - are these different models?
> Yes, even with different CPU architectures.
> How much the "microp" device differs is not clear, but the
> communication protocol is the same.
> > 
> > EC is not a generic purpose component and is tightly coupled with the
> > actual board it is being present on. Unless exactly same board is used
> > in different models (unlikely) then the compatible defines the LEDs and
> > they are not needed in DT.
> So for instance "synology,ds923p-microp", "synology,ds723p-microp" etc.
> ?
> 
> I can do that, but that would be many.

How many is many?

> Having it generic seems more flexible.

Is there firmware for these ECs? If so is it the same or different 
firmware for each device? If the former or the functionality is really 
trivial, then I'd be more comfortable with 1 or a few compatibles. 

Generic means you'll need to add quirk properties when there is some 
difference the OS needs to handle which we'll reject. So stuck with one 
compatible and no way to distinguish different ECs is anything but 
flexible.

Rob

Re: [PATCH v4 1/2] dt-bindings: embedded-controller: Add synology,microp device

[Markus Probst]

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

On Wed, 2026-03-25 at 17:07 -0500, Rob Herring wrote:
> On Sat, Mar 21, 2026 at 01:02:22PM +0000, Markus Probst wrote:
> > On Sat, 2026-03-21 at 13:32 +0100, Krzysztof Kozlowski wrote:
> > > On 21/03/2026 13:17, Markus Probst wrote:
> > > > On Sat, 2026-03-21 at 11:21 +0100, Krzysztof Kozlowski wrote:
> > > > > On Fri, Mar 20, 2026 at 11:09:53PM +0100, Markus Probst wrote:
> > > > > > +
> > > > > > +examples:
> > > > > > +  - |
> > > > > > +    #include <dt-bindings/leds/common.h>
> > > > > > +
> > > > > > +    embedded-controller {
> > > > > > +      compatible = "synology,microp";
> > > > > > +
> > > > > > +      power-led {
> > > > > > +        color = <LED_COLOR_ID_BLUE>;
> > > > > > +        function = LED_FUNCTION_POWER;
> > > > > > +      };
> > > > > > +
> > > > > > +      status-led {
> > > > > > +        color = <LED_COLOR_ID_MULTI>;
> > > > > > +        function = LED_FUNCTION_STATUS;
> > > > > > +      };
> > > > > 
> > > > > Where are other leds? Binding mentions 4.
> > > > > 
> > > > Status and Power leds exist on every Synology NAS model I am aware of.
> > > > But there are models which have additionally a usb or alert led. The
> > > > device nodes for those leds should only be present, if they exist
> > > > physically on the device.
> > > 
> > > Then help me to understand - are these different models?
> > Yes, even with different CPU architectures.
> > How much the "microp" device differs is not clear, but the
> > communication protocol is the same.
> > > 
> > > EC is not a generic purpose component and is tightly coupled with the
> > > actual board it is being present on. Unless exactly same board is used
> > > in different models (unlikely) then the compatible defines the LEDs and
> > > they are not needed in DT.
> > So for instance "synology,ds923p-microp", "synology,ds723p-microp" etc.
> > ?
> > 
> > I can do that, but that would be many.
> 
> How many is many?
Estimated 300.

As a side note: I only have 1 model I can test the driver with.
> 
> > Having it generic seems more flexible.
> 
> Is there firmware for these ECs? If so is it the same or different 
> firmware for each device? If the former or the functionality is really 
> trivial, then I'd be more comfortable with 1 or a few compatibles. 
The firmware is not public and the exact differences between them isn't
documented. The communication protocol is the same though.

> 
> Generic means you'll need to add quirk properties when there is some 
> difference the OS needs to handle which we'll reject. So stuck with one 
> compatible and no way to distinguish different ECs is anything but 
> flexible.
Describing the physical leds that are present on the NAS device are not
quirk properties, at least in my definition.

Thanks
- Markus Probst


> 
> Rob

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

Re: [PATCH v4 1/2] dt-bindings: embedded-controller: Add synology,microp device

[Rob Herring]

On Thu, Mar 26, 2026 at 8:02 AM Markus Probst <markus.probst@posteo.de> wrote:
>
> On Wed, 2026-03-25 at 17:07 -0500, Rob Herring wrote:
> > On Sat, Mar 21, 2026 at 01:02:22PM +0000, Markus Probst wrote:
> > > On Sat, 2026-03-21 at 13:32 +0100, Krzysztof Kozlowski wrote:
> > > > On 21/03/2026 13:17, Markus Probst wrote:
> > > > > On Sat, 2026-03-21 at 11:21 +0100, Krzysztof Kozlowski wrote:
> > > > > > On Fri, Mar 20, 2026 at 11:09:53PM +0100, Markus Probst wrote:
> > > > > > > +
> > > > > > > +examples:
> > > > > > > +  - |
> > > > > > > +    #include <dt-bindings/leds/common.h>
> > > > > > > +
> > > > > > > +    embedded-controller {
> > > > > > > +      compatible = "synology,microp";
> > > > > > > +
> > > > > > > +      power-led {
> > > > > > > +        color = <LED_COLOR_ID_BLUE>;
> > > > > > > +        function = LED_FUNCTION_POWER;
> > > > > > > +      };
> > > > > > > +
> > > > > > > +      status-led {
> > > > > > > +        color = <LED_COLOR_ID_MULTI>;
> > > > > > > +        function = LED_FUNCTION_STATUS;
> > > > > > > +      };
> > > > > >
> > > > > > Where are other leds? Binding mentions 4.
> > > > > >
> > > > > Status and Power leds exist on every Synology NAS model I am aware of.
> > > > > But there are models which have additionally a usb or alert led. The
> > > > > device nodes for those leds should only be present, if they exist
> > > > > physically on the device.
> > > >
> > > > Then help me to understand - are these different models?
> > > Yes, even with different CPU architectures.
> > > How much the "microp" device differs is not clear, but the
> > > communication protocol is the same.
> > > >
> > > > EC is not a generic purpose component and is tightly coupled with the
> > > > actual board it is being present on. Unless exactly same board is used
> > > > in different models (unlikely) then the compatible defines the LEDs and
> > > > they are not needed in DT.
> > > So for instance "synology,ds923p-microp", "synology,ds723p-microp" etc.
> > > ?
> > >
> > > I can do that, but that would be many.
> >
> > How many is many?
> Estimated 300.

Okay, that's a lot and probably safe to say there are not 300
variations of the EC.

> As a side note: I only have 1 model I can test the driver with.
> >
> > > Having it generic seems more flexible.
> >
> > Is there firmware for these ECs? If so is it the same or different
> > firmware for each device? If the former or the functionality is really
> > trivial, then I'd be more comfortable with 1 or a few compatibles.
> The firmware is not public and the exact differences between them isn't
> documented. The communication protocol is the same though.
>
> >
> > Generic means you'll need to add quirk properties when there is some
> > difference the OS needs to handle which we'll reject. So stuck with one
> > compatible and no way to distinguish different ECs is anything but
> > flexible.
> Describing the physical leds that are present on the NAS device are not
> quirk properties, at least in my definition.

That's not what I mean. I mean things like this other device needs
some different timing for power-on/reset or delays between accesses or
some LED control is inverted or some protocol difference... Could be
about anything. The key thing is you have specific enough information
(compatible) to start with that you can handle any issue that comes up
*without* changing the DT.

As you said, you only have 1 device. Make the binding specific to that
1 device. If the next one that comes along can reuse the binding as
it, then great. Nothing to do. If it can't, then it gets its own new
compatible. Strictly speaking we would add a new compatible for each
device, but it's a judgement call that there aren't going to be
differences to handle. In this case, there likely aren't 300 versions
of h/w, the functionality is simple enough, and the functionality is
entirely optional (just a guess). But that's all really your argument
to make.

Rob

Re: [PATCH v4 1/2] dt-bindings: embedded-controller: Add synology,microp device

[Markus Probst]

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

On Thu, 2026-03-26 at 14:36 -0500, Rob Herring wrote:
> On Thu, Mar 26, 2026 at 8:02 AM Markus Probst <markus.probst@posteo.de> wrote:
> > 
> > On Wed, 2026-03-25 at 17:07 -0500, Rob Herring wrote:
> > > On Sat, Mar 21, 2026 at 01:02:22PM +0000, Markus Probst wrote:
> > > > On Sat, 2026-03-21 at 13:32 +0100, Krzysztof Kozlowski wrote:
> > > > > On 21/03/2026 13:17, Markus Probst wrote:
> > > > > > On Sat, 2026-03-21 at 11:21 +0100, Krzysztof Kozlowski wrote:
> > > > > > > On Fri, Mar 20, 2026 at 11:09:53PM +0100, Markus Probst wrote:
> > > > > > > > +
> > > > > > > > +examples:
> > > > > > > > +  - |
> > > > > > > > +    #include <dt-bindings/leds/common.h>
> > > > > > > > +
> > > > > > > > +    embedded-controller {
> > > > > > > > +      compatible = "synology,microp";
> > > > > > > > +
> > > > > > > > +      power-led {
> > > > > > > > +        color = <LED_COLOR_ID_BLUE>;
> > > > > > > > +        function = LED_FUNCTION_POWER;
> > > > > > > > +      };
> > > > > > > > +
> > > > > > > > +      status-led {
> > > > > > > > +        color = <LED_COLOR_ID_MULTI>;
> > > > > > > > +        function = LED_FUNCTION_STATUS;
> > > > > > > > +      };
> > > > > > > 
> > > > > > > Where are other leds? Binding mentions 4.
> > > > > > > 
> > > > > > Status and Power leds exist on every Synology NAS model I am aware of.
> > > > > > But there are models which have additionally a usb or alert led. The
> > > > > > device nodes for those leds should only be present, if they exist
> > > > > > physically on the device.
> > > > > 
> > > > > Then help me to understand - are these different models?
> > > > Yes, even with different CPU architectures.
> > > > How much the "microp" device differs is not clear, but the
> > > > communication protocol is the same.
> > > > > 
> > > > > EC is not a generic purpose component and is tightly coupled with the
> > > > > actual board it is being present on. Unless exactly same board is used
> > > > > in different models (unlikely) then the compatible defines the LEDs and
> > > > > they are not needed in DT.
> > > > So for instance "synology,ds923p-microp", "synology,ds723p-microp" etc.
> > > > ?
> > > > 
> > > > I can do that, but that would be many.
> > > 
> > > How many is many?
> > Estimated 300.
> 
> Okay, that's a lot and probably safe to say there are not 300
> variations of the EC.
> 
> > As a side note: I only have 1 model I can test the driver with.
> > > 
> > > > Having it generic seems more flexible.
> > > 
> > > Is there firmware for these ECs? If so is it the same or different
> > > firmware for each device? If the former or the functionality is really
> > > trivial, then I'd be more comfortable with 1 or a few compatibles.
> > The firmware is not public and the exact differences between them isn't
> > documented. The communication protocol is the same though.
> > 
> > > 
> > > Generic means you'll need to add quirk properties when there is some
> > > difference the OS needs to handle which we'll reject. So stuck with one
> > > compatible and no way to distinguish different ECs is anything but
> > > flexible.
> > Describing the physical leds that are present on the NAS device are not
> > quirk properties, at least in my definition.
> 
> That's not what I mean. I mean things like this other device needs
> some different timing for power-on/reset or delays between accesses or
> some LED control is inverted or some protocol difference... Could be
> about anything. The key thing is you have specific enough information
> (compatible) to start with that you can handle any issue that comes up
> *without* changing the DT.
> 
> As you said, you only have 1 device. Make the binding specific to that
> 1 device. If the next one that comes along can reuse the binding as
> it, then great. Nothing to do. If it can't, then it gets its own new
> compatible. Strictly speaking we would add a new compatible for each
> device, but it's a judgement call that there aren't going to be
> differences to handle. In this case, there likely aren't 300 versions
> of h/w, the functionality is simple enough, and the functionality is
> entirely optional (just a guess). But that's all really your argument
> to make.
I think I will go with different compatible, but instead of 1 I will
compile a small list of devices, which:
- are most similar to my testing device
- have unique functionality (like having additionally a "cpu fan" or
similar). This way I can implement every functionality in the driver,
which makes it easier to add devices in the future.
- I know are owned by people who like to experiment with their device
(including my device)

Thanks
- Markus Probst

> 
> Rob

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