public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Introduce Synology Microp driver
@ 2026-03-08 18:41 Markus Probst
  2026-03-08 18:41 ` [PATCH v2 1/2] dt-bindings: mfd: Add synology,microp device Markus Probst
  2026-03-08 18:41 ` [PATCH v2 2/2] mfd: Add initial synology microp driver Markus Probst
  0 siblings, 2 replies; 21+ messages in thread
From: Markus Probst @ 2026-03-08 18:41 UTC (permalink / raw)
  To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich
  Cc: 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/20260207-rust_leds-v12-0-fdb518417b75@posteo.de/
[2] https://lore.kernel.org/rust-for-linux/20260306-rust_serdev-v2-0-e9b23b42b255@posteo.de/

Signed-off-by: Markus Probst <markus.probst@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: mfd: Add synology,microp device
      mfd: Add initial synology microp driver

 .../devicetree/bindings/mfd/synology,microp.yaml   |  49 ++++
 MAINTAINERS                                        |   6 +
 drivers/mfd/Kconfig                                |   2 +
 drivers/mfd/Makefile                               |   2 +
 drivers/mfd/synology_microp/Kconfig                |  14 ++
 drivers/mfd/synology_microp/Makefile               |   2 +
 drivers/mfd/synology_microp/TODO                   |   7 +
 drivers/mfd/synology_microp/command.rs             |  50 ++++
 drivers/mfd/synology_microp/led.rs                 | 275 +++++++++++++++++++++
 drivers/mfd/synology_microp/synology_microp.rs     |  82 ++++++
 rust/uapi/uapi_helper.h                            |   2 +
 11 files changed, 491 insertions(+)
---
base-commit: 9fad1d148df6f36105159c2503d0ecb1397bc89a
change-id: 20260306-synology_microp_initial-0f7dac7b7496
prerequisite-change-id: 20251217-rust_serdev-ee5481e9085c:v2
prerequisite-patch-id: 52b17274481cc770c257d8f95335293eca32a2c5
prerequisite-patch-id: 680c0509490833f416b17eb3b918bb829ce7b90f
prerequisite-patch-id: 37016d6c07f0da898be02bf30110e8c0a30025d2
prerequisite-patch-id: 3dfc1f7e5ecd3e0dd65d676aeb16f55260847b25
prerequisite-change-id: 20251114-rust_leds-a959f7c2f7f9:v12
prerequisite-patch-id: 42c445ef6981e3a3740dbaaf307f4b810042e46f
prerequisite-patch-id: 90c7b200cca722a592353885e21af069101c4e09
prerequisite-patch-id: c664a52faa3d47000d252eb7603c9c08382e868a


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 1/2] dt-bindings: mfd: Add synology,microp device
  2026-03-08 18:41 [PATCH v2 0/2] Introduce Synology Microp driver Markus Probst
@ 2026-03-08 18:41 ` Markus Probst
  2026-03-09  7:17   ` Krzysztof Kozlowski
  2026-03-08 18:41 ` [PATCH v2 2/2] mfd: Add initial synology microp driver Markus Probst
  1 sibling, 1 reply; 21+ messages in thread
From: Markus Probst @ 2026-03-08 18:41 UTC (permalink / raw)
  To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich
  Cc: 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.

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

diff --git a/Documentation/devicetree/bindings/mfd/synology,microp.yaml b/Documentation/devicetree/bindings/mfd/synology,microp.yaml
new file mode 100644
index 000000000000..60609bb19b66
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/synology,microp.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/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 devices contain a microcontroller on their device to control
+  certain leds, fan speeds, a beeper, to properly handle system shutdown
+  and reboot, buttons and fan failures.
+
+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>
+
+    mcu {
+      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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-08 18:41 [PATCH v2 0/2] Introduce Synology Microp driver Markus Probst
  2026-03-08 18:41 ` [PATCH v2 1/2] dt-bindings: mfd: Add synology,microp device Markus Probst
@ 2026-03-08 18:41 ` Markus Probst
  2026-03-08 18:55   ` Greg Kroah-Hartman
  2026-03-08 18:56   ` Greg Kroah-Hartman
  1 sibling, 2 replies; 21+ messages in thread
From: Markus Probst @ 2026-03-08 18:41 UTC (permalink / raw)
  To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich
  Cc: 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.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 MAINTAINERS                                    |   6 +
 drivers/mfd/Kconfig                            |   2 +
 drivers/mfd/Makefile                           |   2 +
 drivers/mfd/synology_microp/Kconfig            |  14 ++
 drivers/mfd/synology_microp/Makefile           |   2 +
 drivers/mfd/synology_microp/TODO               |   7 +
 drivers/mfd/synology_microp/command.rs         |  50 +++++
 drivers/mfd/synology_microp/led.rs             | 275 +++++++++++++++++++++++++
 drivers/mfd/synology_microp/synology_microp.rs |  82 ++++++++
 rust/uapi/uapi_helper.h                        |   2 +
 10 files changed, 442 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index e9e83ab552c7..092cd9e8a730 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25550,6 +25550,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/mfd/synology,microp.yaml
+F:	drivers/mfd/synology_microp/
+
 SYNOPSYS ARC ARCHITECTURE
 M:	Vineet Gupta <vgupta@kernel.org>
 L:	linux-snps-arc@lists.infradead.org
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 7192c9d1d268..bc269719749f 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2580,5 +2580,7 @@ config MFD_MAX7360
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+source "drivers/mfd/synology_microp/Kconfig"
+
 endmenu
 endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e75e8045c28a..0a6fa33d5c35 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -304,3 +304,5 @@ obj-$(CONFIG_MFD_RSMU_SPI)	+= rsmu_spi.o rsmu_core.o
 obj-$(CONFIG_MFD_UPBOARD_FPGA)	+= upboard-fpga.o
 
 obj-$(CONFIG_MFD_LOONGSON_SE)	+= loongson-se.o
+
+obj-$(CONFIG_MFD_SYNOLOGY_MICROP)	+= synology_microp/
diff --git a/drivers/mfd/synology_microp/Kconfig b/drivers/mfd/synology_microp/Kconfig
new file mode 100644
index 000000000000..4bbbcf0b6e94
--- /dev/null
+++ b/drivers/mfd/synology_microp/Kconfig
@@ -0,0 +1,14 @@
+
+config MFD_SYNOLOGY_MICROP
+	tristate "Synology Microp driver"
+	depends on RUST
+	depends on SERIAL_DEV_BUS
+	depends on LEDS_CLASS && LEDS_CLASS_MULTICOLOR
+	default n
+	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/mfd/synology_microp/Makefile b/drivers/mfd/synology_microp/Makefile
new file mode 100644
index 000000000000..d762cada20c9
--- /dev/null
+++ b/drivers/mfd/synology_microp/Makefile
@@ -0,0 +1,2 @@
+
+obj-y	+= synology_microp.o
diff --git a/drivers/mfd/synology_microp/TODO b/drivers/mfd/synology_microp/TODO
new file mode 100644
index 000000000000..1961a33115db
--- /dev/null
+++ b/drivers/mfd/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/mfd/synology_microp/command.rs b/drivers/mfd/synology_microp/command.rs
new file mode 100644
index 000000000000..78f82a86f1b2
--- /dev/null
+++ b/drivers/mfd/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 {
+                Command::PowerLed(led::State::On) => &[0x34],
+                Command::PowerLed(led::State::Blink) => &[0x35],
+                Command::PowerLed(led::State::Off) => &[0x36],
+
+                Command::StatusLed(_, led::State::Off) => &[0x37],
+                Command::StatusLed(led::StatusLedColor::Green, led::State::On) => &[0x38],
+                Command::StatusLed(led::StatusLedColor::Green, led::State::Blink) => &[0x39],
+                Command::StatusLed(led::StatusLedColor::Orange, led::State::On) => &[0x3A],
+                Command::StatusLed(led::StatusLedColor::Orange, led::State::Blink) => &[0x3B],
+
+                Command::AlertLed(led::State::On) => &[0x4C, 0x41, 0x31],
+                Command::AlertLed(led::State::Blink) => &[0x4C, 0x41, 0x32],
+                Command::AlertLed(led::State::Off) => &[0x4C, 0x41, 0x33],
+
+                Command::UsbLed(led::State::On) => &[0x40],
+                Command::UsbLed(led::State::Blink) => &[0x41],
+                Command::UsbLed(led::State::Off) => &[0x42],
+            },
+            serdev::Timeout::Max,
+        )?;
+        dev.wait_until_sent(serdev::Timeout::Max);
+        Ok(())
+    }
+}
diff --git a/drivers/mfd/synology_microp/led.rs b/drivers/mfd/synology_microp/led.rs
new file mode 100644
index 000000000000..28a765b1e6c2
--- /dev/null
+++ b/drivers/mfd/synology_microp/led.rs
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use core::sync::atomic::{
+    AtomicBool,
+    Ordering, //
+};
+
+use kernel::{
+    device::{
+        property::FwNode,
+        Bound, //
+    },
+    devres::Devres,
+    error::Error,
+    led::{self, MultiColorSubLed},
+    macros::vtable,
+    prelude::*,
+    serdev,
+    types::ARef, //
+};
+
+use crate::command::Command;
+
+pub(crate) struct SynologyMicropLedHandler {
+    blink: AtomicBool,
+    map: fn(State) -> Command,
+}
+
+pub(crate) struct SynologyMicropStatusLedHandler {
+    blink: AtomicBool,
+}
+
+#[derive(Copy, Clone)]
+pub(crate) enum State {
+    On,
+    Blink,
+    Off,
+}
+
+#[derive(Copy, Clone)]
+pub(crate) enum StatusLedColor {
+    Green,
+    Orange,
+}
+
+impl SynologyMicropLedHandler {
+    fn register_by_fwnode<'a>(
+        parent: &'a serdev::Device<Bound>,
+        default_trigger: &'static CStr,
+        brightness: u32,
+        color: led::Color,
+        map: fn(State) -> Command,
+        fwnode: Option<ARef<FwNode>>,
+    ) -> impl PinInit<Devres<led::Device<Self>>, Error> + 'a {
+        led::DeviceBuilder::new()
+            .fwnode(fwnode)
+            .default_trigger(default_trigger)
+            .initial_brightness(brightness)
+            .devicename(c"synology-microp")
+            .color(color)
+            .build(
+                parent,
+                Ok(Self {
+                    blink: AtomicBool::new(true),
+                    map,
+                }),
+            )
+    }
+
+    fn register<'a>(
+        parent: &'a serdev::Device<Bound>,
+        fwnode_child_name: &'static CStr,
+        default_trigger: &'static CStr,
+        brightness: u32,
+        color: led::Color,
+        map: fn(State) -> Command,
+    ) -> impl PinInit<Devres<led::Device<Self>>, Error> + 'a {
+        Self::register_by_fwnode(
+            parent,
+            default_trigger,
+            brightness,
+            color,
+            map,
+            parent
+                .as_ref()
+                .fwnode()
+                .and_then(|fwnode| fwnode.get_child_by_name(fwnode_child_name)),
+        )
+    }
+
+    fn register_optional<'a>(
+        parent: &'a serdev::Device<Bound>,
+        fwnode_child_name: &'static CStr,
+        default_trigger: &'static CStr,
+        brightness: u32,
+        color: led::Color,
+        map: fn(State) -> Command,
+    ) -> Option<impl PinInit<Devres<led::Device<Self>>, Error> + 'a> {
+        parent
+            .as_ref()
+            .fwnode()
+            .and_then(|fwnode| fwnode.get_child_by_name(fwnode_child_name))
+            .map(|fwnode| {
+                Self::register_by_fwnode(
+                    parent,
+                    default_trigger,
+                    brightness,
+                    color,
+                    map,
+                    Some(fwnode),
+                )
+            })
+    }
+
+    pub(crate) fn register_power<'a>(
+        parent: &'a serdev::Device<Bound>,
+    ) -> impl PinInit<Devres<led::Device<Self>>, Error> + 'a {
+        Self::register(
+            parent,
+            c"power-led",
+            c"timer",
+            1,
+            led::Color::Blue,
+            Command::PowerLed,
+        )
+    }
+
+    pub(crate) fn register_alert<'a>(
+        parent: &'a serdev::Device<Bound>,
+    ) -> Option<impl PinInit<Devres<led::Device<Self>>, Error> + 'a> {
+        Self::register_optional(
+            parent,
+            c"alert-led",
+            c"none",
+            0,
+            led::Color::Orange,
+            Command::AlertLed,
+        )
+    }
+
+    pub(crate) fn register_usb<'a>(
+        parent: &'a serdev::Device<Bound>,
+    ) -> Option<impl PinInit<Devres<led::Device<Self>>, Error> + 'a> {
+        Self::register_optional(
+            parent,
+            c"usb-led",
+            c"none",
+            0,
+            led::Color::Green,
+            Command::UsbLed,
+        )
+    }
+}
+
+#[vtable]
+impl led::LedOps for SynologyMicropLedHandler {
+    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<()> {
+        (self.map)(if brightness == 0 {
+            self.blink.store(false, Ordering::Relaxed);
+            State::Off
+        } else if self.blink.load(Ordering::Relaxed) {
+            State::Blink
+        } else {
+            State::On
+        })
+        .write(dev)
+    }
+
+    fn blink_set(
+        &self,
+        dev: &Self::Bus,
+        _classdev: &led::Device<Self>,
+        delay_on: &mut usize,
+        delay_off: &mut usize,
+    ) -> Result<()> {
+        *delay_on = 167;
+        *delay_off = 167;
+
+        self.blink.store(true, Ordering::Relaxed);
+        (self.map)(State::Blink).write(dev)
+    }
+}
+
+impl SynologyMicropStatusLedHandler {
+    pub(crate) fn register(
+        parent: &serdev::Device<Bound>,
+    ) -> impl PinInit<Devres<led::MultiColorDevice<Self>>, Error> + '_ {
+        const SUBLEDS: &[MultiColorSubLed] = &[
+            MultiColorSubLed::new(led::Color::Green).initial_intensity(1),
+            MultiColorSubLed::new(led::Color::Orange),
+        ];
+
+        led::DeviceBuilder::new()
+            .fwnode(
+                parent
+                    .as_ref()
+                    .fwnode()
+                    .and_then(|fwnode| fwnode.get_child_by_name(c"status-led")),
+            )
+            .devicename(c"synology-microp")
+            .color(led::Color::Multi)
+            .build_multicolor(
+                parent,
+                Ok(SynologyMicropStatusLedHandler {
+                    blink: AtomicBool::new(false),
+                }),
+                SUBLEDS,
+            )
+    }
+}
+
+#[vtable]
+impl led::LedOps for SynologyMicropStatusLedHandler {
+    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<()> {
+        if brightness == 0 {
+            self.blink.store(false, Ordering::Relaxed);
+        }
+
+        let (color, subled_brightness) = if classdev.subleds()[1].brightness == 0 {
+            (StatusLedColor::Green, classdev.subleds()[0].brightness)
+        } else {
+            (StatusLedColor::Orange, classdev.subleds()[1].brightness)
+        };
+
+        if subled_brightness == 0 {
+            Command::StatusLed(color, State::Off)
+        } else if self.blink.load(Ordering::Relaxed) {
+            Command::StatusLed(color, State::Blink)
+        } else {
+            Command::StatusLed(color, State::On)
+        }
+        .write(dev)
+    }
+
+    fn blink_set(
+        &self,
+        dev: &Self::Bus,
+        classdev: &led::MultiColorDevice<Self>,
+        delay_on: &mut usize,
+        delay_off: &mut usize,
+    ) -> Result<()> {
+        *delay_on = 167;
+        *delay_off = 167;
+
+        self.blink.store(true, Ordering::Relaxed);
+
+        let color = if classdev.subleds()[1].brightness == 0 {
+            StatusLedColor::Green
+        } else {
+            StatusLedColor::Orange
+        };
+
+        Command::StatusLed(color, State::Blink).write(dev)
+    }
+}
diff --git a/drivers/mfd/synology_microp/synology_microp.rs b/drivers/mfd/synology_microp/synology_microp.rs
new file mode 100644
index 000000000000..e92de7da3e46
--- /dev/null
+++ b/drivers/mfd/synology_microp/synology_microp.rs
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Synology Microp driver
+
+use kernel::{
+    device,
+    devres::{self, Devres},
+    of,
+    prelude::*,
+    serdev, //
+};
+use pin_init::pin_init_scope;
+
+use crate::{
+    led::{
+        SynologyMicropLedHandler,
+        SynologyMicropStatusLedHandler, //
+    }, //
+};
+
+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",
+    params: {
+        check_fan: i32 {
+            default: 1,
+            description: "Check for cpu fan failures",
+        },
+    },
+}
+
+#[pin_data]
+struct SynologyMicropDriver {
+    #[pin]
+    power_led: Devres<kernel::led::Device<SynologyMicropLedHandler>>,
+    #[pin]
+    status_led: Devres<kernel::led::MultiColorDevice<SynologyMicropStatusLedHandler>>,
+}
+
+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<kernel::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 _ = dev.set_baudrate(9600);
+            dev.set_flow_control(false);
+            dev.set_parity(serdev::Parity::None)?;
+
+            // TODO: Replace with Option field on SynologyMicropDriver once
+            // https://github.com/Rust-for-Linux/pin-init/issues/59 has been resolved.
+            if let Some(alert_led) = SynologyMicropLedHandler::register_alert(dev) {
+                devres::register(dev.as_ref(), alert_led, GFP_KERNEL)?;
+            }
+            if let Some(usb_led) = SynologyMicropLedHandler::register_usb(dev) {
+                devres::register(dev.as_ref(), usb_led, GFP_KERNEL)?;
+            }
+
+            Ok(try_pin_init!(Self {
+                power_led <- SynologyMicropLedHandler::register_power(dev),
+                status_led <- SynologyMicropStatusLedHandler::register(dev),
+            }))
+        })
+    }
+}
diff --git a/rust/uapi/uapi_helper.h b/rust/uapi/uapi_helper.h
index 06d7d1a2e8da..94b6c1b59e56 100644
--- a/rust/uapi/uapi_helper.h
+++ b/rust/uapi/uapi_helper.h
@@ -14,3 +14,5 @@
 #include <uapi/linux/mdio.h>
 #include <uapi/linux/mii.h>
 #include <uapi/linux/ethtool.h>
+#include <uapi/linux/serial_reg.h>
+

-- 
2.52.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-08 18:41 ` [PATCH v2 2/2] mfd: Add initial synology microp driver Markus Probst
@ 2026-03-08 18:55   ` Greg Kroah-Hartman
  2026-03-08 19:15     ` Markus Probst
  2026-03-08 18:56   ` Greg Kroah-Hartman
  1 sibling, 1 reply; 21+ messages in thread
From: Greg Kroah-Hartman @ 2026-03-08 18:55 UTC (permalink / raw)
  To: Markus Probst
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, devicetree, linux-kernel, rust-for-linux

On Sun, Mar 08, 2026 at 06:41:20PM +0000, 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.

Why is this a mfd device?  Shouldn't it be an aux device?

But this is just a serial port connection, so why is a kernel driver
needed at all?

> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
>  MAINTAINERS                                    |   6 +
>  drivers/mfd/Kconfig                            |   2 +
>  drivers/mfd/Makefile                           |   2 +
>  drivers/mfd/synology_microp/Kconfig            |  14 ++
>  drivers/mfd/synology_microp/Makefile           |   2 +
>  drivers/mfd/synology_microp/TODO               |   7 +
>  drivers/mfd/synology_microp/command.rs         |  50 +++++
>  drivers/mfd/synology_microp/led.rs             | 275 +++++++++++++++++++++++++
>  drivers/mfd/synology_microp/synology_microp.rs |  82 ++++++++
>  rust/uapi/uapi_helper.h                        |   2 +
>  10 files changed, 442 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e9e83ab552c7..092cd9e8a730 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -25550,6 +25550,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/mfd/synology,microp.yaml
> +F:	drivers/mfd/synology_microp/
> +
>  SYNOPSYS ARC ARCHITECTURE
>  M:	Vineet Gupta <vgupta@kernel.org>
>  L:	linux-snps-arc@lists.infradead.org
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 7192c9d1d268..bc269719749f 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2580,5 +2580,7 @@ config MFD_MAX7360
>  	  additional drivers must be enabled in order to use the functionality
>  	  of the device.
>  
> +source "drivers/mfd/synology_microp/Kconfig"
> +
>  endmenu
>  endif
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e75e8045c28a..0a6fa33d5c35 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -304,3 +304,5 @@ obj-$(CONFIG_MFD_RSMU_SPI)	+= rsmu_spi.o rsmu_core.o
>  obj-$(CONFIG_MFD_UPBOARD_FPGA)	+= upboard-fpga.o
>  
>  obj-$(CONFIG_MFD_LOONGSON_SE)	+= loongson-se.o
> +
> +obj-$(CONFIG_MFD_SYNOLOGY_MICROP)	+= synology_microp/
> diff --git a/drivers/mfd/synology_microp/Kconfig b/drivers/mfd/synology_microp/Kconfig
> new file mode 100644
> index 000000000000..4bbbcf0b6e94
> --- /dev/null
> +++ b/drivers/mfd/synology_microp/Kconfig
> @@ -0,0 +1,14 @@
> +
> +config MFD_SYNOLOGY_MICROP
> +	tristate "Synology Microp driver"
> +	depends on RUST
> +	depends on SERIAL_DEV_BUS

We don't have rust serdev bindings yet, but if we do, shouldn't you just
depend on them instead of two different things here?


> +	depends on LEDS_CLASS && LEDS_CLASS_MULTICOLOR
> +	default n

n is always the default, no need to say it again :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-08 18:41 ` [PATCH v2 2/2] mfd: Add initial synology microp driver Markus Probst
  2026-03-08 18:55   ` Greg Kroah-Hartman
@ 2026-03-08 18:56   ` Greg Kroah-Hartman
  2026-03-08 19:23     ` Markus Probst
  1 sibling, 1 reply; 21+ messages in thread
From: Greg Kroah-Hartman @ 2026-03-08 18:56 UTC (permalink / raw)
  To: Markus Probst
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, devicetree, linux-kernel, rust-for-linux

On Sun, Mar 08, 2026 at 06:41:20PM +0000, Markus Probst wrote:
> +kernel::module_serdev_device_driver! {
> +    type: SynologyMicropDriver,
> +    name: "synology_microp",
> +    authors: ["Markus Probst <markus.probst@posteo.de>"],
> +    description: "Synology Microp driver",
> +    license: "GPL v2",
> +    params: {
> +        check_fan: i32 {
> +            default: 1,
> +            description: "Check for cpu fan failures",
> +        },
> +    },

This is not the 1990's, please do not add new module parameters for no
good reason.  This should be dynamic (why would you NOT want to check
for cpu fan failures?) and per-device, not per-module.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-08 18:55   ` Greg Kroah-Hartman
@ 2026-03-08 19:15     ` Markus Probst
  2026-03-09  5:56       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 21+ messages in thread
From: Markus Probst @ 2026-03-08 19:15 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, devicetree, linux-kernel, rust-for-linux

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

On Sun, 2026-03-08 at 19:55 +0100, Greg Kroah-Hartman wrote:
> On Sun, Mar 08, 2026 at 06:41:20PM +0000, 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.
> 
> Why is this a mfd device?  Shouldn't it be an aux device?
> 
> But this is just a serial port connection, so why is a kernel driver
> needed at all?
I am not sure what you mean.

It has multiple functions (leds, hwmon, power/reset, input etc.) and
does is a multifunction device (mfd).

It does not however use mfd-core or anything from the auxiliary device
and instead implements its functionality directly in this driver.

> 
> > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > ---
> >  MAINTAINERS                                    |   6 +
> >  drivers/mfd/Kconfig                            |   2 +
> >  drivers/mfd/Makefile                           |   2 +
> >  drivers/mfd/synology_microp/Kconfig            |  14 ++
> >  drivers/mfd/synology_microp/Makefile           |   2 +
> >  drivers/mfd/synology_microp/TODO               |   7 +
> >  drivers/mfd/synology_microp/command.rs         |  50 +++++
> >  drivers/mfd/synology_microp/led.rs             | 275 +++++++++++++++++++++++++
> >  drivers/mfd/synology_microp/synology_microp.rs |  82 ++++++++
> >  rust/uapi/uapi_helper.h                        |   2 +
> >  10 files changed, 442 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index e9e83ab552c7..092cd9e8a730 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -25550,6 +25550,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/mfd/synology,microp.yaml
> > +F:	drivers/mfd/synology_microp/
> > +
> >  SYNOPSYS ARC ARCHITECTURE
> >  M:	Vineet Gupta <vgupta@kernel.org>
> >  L:	linux-snps-arc@lists.infradead.org
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 7192c9d1d268..bc269719749f 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -2580,5 +2580,7 @@ config MFD_MAX7360
> >  	  additional drivers must be enabled in order to use the functionality
> >  	  of the device.
> >  
> > +source "drivers/mfd/synology_microp/Kconfig"
> > +
> >  endmenu
> >  endif
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index e75e8045c28a..0a6fa33d5c35 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -304,3 +304,5 @@ obj-$(CONFIG_MFD_RSMU_SPI)	+= rsmu_spi.o rsmu_core.o
> >  obj-$(CONFIG_MFD_UPBOARD_FPGA)	+= upboard-fpga.o
> >  
> >  obj-$(CONFIG_MFD_LOONGSON_SE)	+= loongson-se.o
> > +
> > +obj-$(CONFIG_MFD_SYNOLOGY_MICROP)	+= synology_microp/
> > diff --git a/drivers/mfd/synology_microp/Kconfig b/drivers/mfd/synology_microp/Kconfig
> > new file mode 100644
> > index 000000000000..4bbbcf0b6e94
> > --- /dev/null
> > +++ b/drivers/mfd/synology_microp/Kconfig
> > @@ -0,0 +1,14 @@
> > +
> > +config MFD_SYNOLOGY_MICROP
> > +	tristate "Synology Microp driver"
> > +	depends on RUST
> > +	depends on SERIAL_DEV_BUS
> 
> We don't have rust serdev bindings yet, but if we do, shouldn't you just
> depend on them instead of two different things here?
I will add a `RUST_SERDEV_ABSTRACTIONS` Kconfig entry in the next
serdev rust abstraction patch revision then.
> 
> 
> > +	depends on LEDS_CLASS && LEDS_CLASS_MULTICOLOR
> > +	default n
> 
> n is always the default, no need to say it again :)
I took some inspiration from the NOVA_CORE Kconfig entry, since that
driver is work in progress too.

> 
> thanks,
> 
> greg k-h

Thanks
- Markus Probst

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-08 18:56   ` Greg Kroah-Hartman
@ 2026-03-08 19:23     ` Markus Probst
  2026-03-09  5:55       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 21+ messages in thread
From: Markus Probst @ 2026-03-08 19:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, devicetree, linux-kernel, rust-for-linux

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

On Sun, 2026-03-08 at 19:56 +0100, Greg Kroah-Hartman wrote:
> On Sun, Mar 08, 2026 at 06:41:20PM +0000, Markus Probst wrote:
> > +kernel::module_serdev_device_driver! {
> > +    type: SynologyMicropDriver,
> > +    name: "synology_microp",
> > +    authors: ["Markus Probst <markus.probst@posteo.de>"],
> > +    description: "Synology Microp driver",
> > +    license: "GPL v2",
> > +    params: {
> > +        check_fan: i32 {
> > +            default: 1,
> > +            description: "Check for cpu fan failures",
> > +        },
> > +    },
> 
> This is not the 1990's, please do not add new module parameters for no
> good reason.  This should be dynamic and per-device, not per-module.
agreed.

> why would you NOT want to check for cpu fan failures?

Because it is also triggered at low fan speeds, even if they are
intentionally set by the driver. While the parameter is 1, the driver
would enable those checks for fan failures and would prevent low fan
speeds. The parameter would allow it to disable this check and allow
for low fan speeds.

> 
> thanks,
> 
> greg k-h


Thanks
- Markus Probst

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-08 19:23     ` Markus Probst
@ 2026-03-09  5:55       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2026-03-09  5:55 UTC (permalink / raw)
  To: Markus Probst
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, devicetree, linux-kernel, rust-for-linux

On Sun, Mar 08, 2026 at 07:23:25PM +0000, Markus Probst wrote:
> On Sun, 2026-03-08 at 19:56 +0100, Greg Kroah-Hartman wrote:
> > On Sun, Mar 08, 2026 at 06:41:20PM +0000, Markus Probst wrote:
> > > +kernel::module_serdev_device_driver! {
> > > +    type: SynologyMicropDriver,
> > > +    name: "synology_microp",
> > > +    authors: ["Markus Probst <markus.probst@posteo.de>"],
> > > +    description: "Synology Microp driver",
> > > +    license: "GPL v2",
> > > +    params: {
> > > +        check_fan: i32 {
> > > +            default: 1,
> > > +            description: "Check for cpu fan failures",
> > > +        },
> > > +    },
> > 
> > This is not the 1990's, please do not add new module parameters for no
> > good reason.  This should be dynamic and per-device, not per-module.
> agreed.
> 
> > why would you NOT want to check for cpu fan failures?
> 
> Because it is also triggered at low fan speeds, even if they are
> intentionally set by the driver. While the parameter is 1, the driver
> would enable those checks for fan failures and would prevent low fan
> speeds. The parameter would allow it to disable this check and allow
> for low fan speeds.

That sounds like something that the driver itself can do on its own,
don't force a user to make a choice that they really shouldn't be
making.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-08 19:15     ` Markus Probst
@ 2026-03-09  5:56       ` Greg Kroah-Hartman
  2026-03-09  9:43         ` Lee Jones
  2026-03-09 12:52         ` Markus Probst
  0 siblings, 2 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2026-03-09  5:56 UTC (permalink / raw)
  To: Markus Probst
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, devicetree, linux-kernel, rust-for-linux

On Sun, Mar 08, 2026 at 07:15:16PM +0000, Markus Probst wrote:
> On Sun, 2026-03-08 at 19:55 +0100, Greg Kroah-Hartman wrote:
> > On Sun, Mar 08, 2026 at 06:41:20PM +0000, 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.
> > 
> > Why is this a mfd device?  Shouldn't it be an aux device?
> > 
> > But this is just a serial port connection, so why is a kernel driver
> > needed at all?
> I am not sure what you mean.

Can't this just be controlled from userspace over the tty device to the
uart this device uses?  Why is a kernel driver needed at all?

> It has multiple functions (leds, hwmon, power/reset, input etc.) and
> does is a multifunction device (mfd).
> 
> It does not however use mfd-core or anything from the auxiliary device
> and instead implements its functionality directly in this driver.

If it does not use mfd-core, then it should not be in drivers/mfd/
right?  Instead, use the aux bus code to split this up into different
devices and attach them that way, as that's what the aux bus code was
created for.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: mfd: Add synology,microp device
  2026-03-08 18:41 ` [PATCH v2 1/2] dt-bindings: mfd: Add synology,microp device Markus Probst
@ 2026-03-09  7:17   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 21+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-09  7:17 UTC (permalink / raw)
  To: Markus Probst, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich
  Cc: devicetree, linux-kernel, rust-for-linux

On 08/03/2026 19:41, 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.
> 
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
>  .../devicetree/bindings/mfd/synology,microp.yaml   | 49 ++++++++++++++++++++++

This is not an "MFD" device.

...

> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/leds/common.h>
> +
> +    mcu {

Please read previous comments.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-09  5:56       ` Greg Kroah-Hartman
@ 2026-03-09  9:43         ` Lee Jones
  2026-03-09 12:52         ` Markus Probst
  1 sibling, 0 replies; 21+ messages in thread
From: Lee Jones @ 2026-03-09  9:43 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Markus Probst, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, devicetree, linux-kernel, rust-for-linux

On Mon, 09 Mar 2026, Greg Kroah-Hartman wrote:

> On Sun, Mar 08, 2026 at 07:15:16PM +0000, Markus Probst wrote:
> > On Sun, 2026-03-08 at 19:55 +0100, Greg Kroah-Hartman wrote:
> > > On Sun, Mar 08, 2026 at 06:41:20PM +0000, 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.
> > > 
> > > Why is this a mfd device?  Shouldn't it be an aux device?
> > > 
> > > But this is just a serial port connection, so why is a kernel driver
> > > needed at all?
> > I am not sure what you mean.
> 
> Can't this just be controlled from userspace over the tty device to the
> uart this device uses?  Why is a kernel driver needed at all?
> 
> > It has multiple functions (leds, hwmon, power/reset, input etc.) and
> > does is a multifunction device (mfd).
> > 
> > It does not however use mfd-core or anything from the auxiliary device
> > and instead implements its functionality directly in this driver.
> 
> If it does not use mfd-core, then it should not be in drivers/mfd/
> right?  Instead, use the aux bus code to split this up into different
> devices and attach them that way, as that's what the aux bus code was
> created for.

Correct.

If the MFD APIs are not used, your device is not a Linux MFD.

MFD is not a dumping ground for devices with more than one function.

Please place all of the relevant pieces into their respective subsystems.

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-09  5:56       ` Greg Kroah-Hartman
  2026-03-09  9:43         ` Lee Jones
@ 2026-03-09 12:52         ` Markus Probst
  2026-03-09 13:07           ` Greg Kroah-Hartman
  2026-03-09 13:32           ` Danilo Krummrich
  1 sibling, 2 replies; 21+ messages in thread
From: Markus Probst @ 2026-03-09 12:52 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, devicetree, linux-kernel, rust-for-linux

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

On Mon, 2026-03-09 at 06:56 +0100, Greg Kroah-Hartman wrote:
> On Sun, Mar 08, 2026 at 07:15:16PM +0000, Markus Probst wrote:
> > On Sun, 2026-03-08 at 19:55 +0100, Greg Kroah-Hartman wrote:
> > > On Sun, Mar 08, 2026 at 06:41:20PM +0000, 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.
> > > 
> > > Why is this a mfd device?  Shouldn't it be an aux device?
> > > 
> > > But this is just a serial port connection, so why is a kernel driver
> > > needed at all?
> > I am not sure what you mean.
> 
> Can't this just be controlled from userspace over the tty device to the
> uart this device uses?  Why is a kernel driver needed at all?
Like with any other bus device, it can be controlled by userspace.

But the kernel already provides the necessary userspace interfaces for
leds, hwmon, input etc. for any userspace application to access.

Furthermore it is required for proper shutdown and reboot, which is a
kernel task.

On arm devices, it completely takes care of the poweroff and reboot.
There is already a driver here drivers/power/reset/qnap-poweroff.c,
which seems to be primarily developed for QNAP, but works for Synology
too.

On x86 devices, is uses ACPI Sleep, but must still announce the
poweroff / reboot prior to the firmware call to the device for proper
shutdown / reboot. There is no existing driver that takes care of this
yet.

> 
> > It has multiple functions (leds, hwmon, power/reset, input etc.) and
> > does is a multifunction device (mfd).
> > 
> > It does not however use mfd-core or anything from the auxiliary device
> > and instead implements its functionality directly in this driver.
> 
> If it does not use mfd-core, then it should not be in drivers/mfd/
> right?  Instead, use the aux bus code to split this up into different
> devices and attach them that way, as that's what the aux bus code was
> created for.
Yes. I will split it into multiple drivers using the aux bus in the
next revision.

> 
> thanks,
> 
> greg k-h

Thanks
- Markus Probst

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-09 12:52         ` Markus Probst
@ 2026-03-09 13:07           ` Greg Kroah-Hartman
  2026-03-09 13:34             ` Markus Probst
  2026-03-09 13:32           ` Danilo Krummrich
  1 sibling, 1 reply; 21+ messages in thread
From: Greg Kroah-Hartman @ 2026-03-09 13:07 UTC (permalink / raw)
  To: Markus Probst
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, devicetree, linux-kernel, rust-for-linux

On Mon, Mar 09, 2026 at 12:52:26PM +0000, Markus Probst wrote:
> On Mon, 2026-03-09 at 06:56 +0100, Greg Kroah-Hartman wrote:
> > On Sun, Mar 08, 2026 at 07:15:16PM +0000, Markus Probst wrote:
> > > On Sun, 2026-03-08 at 19:55 +0100, Greg Kroah-Hartman wrote:
> > > > On Sun, Mar 08, 2026 at 06:41:20PM +0000, 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.
> > > > 
> > > > Why is this a mfd device?  Shouldn't it be an aux device?
> > > > 
> > > > But this is just a serial port connection, so why is a kernel driver
> > > > needed at all?
> > > I am not sure what you mean.
> > 
> > Can't this just be controlled from userspace over the tty device to the
> > uart this device uses?  Why is a kernel driver needed at all?
> Like with any other bus device, it can be controlled by userspace.

Great, then usually that means it should not be a kernel driver :)

> But the kernel already provides the necessary userspace interfaces for
> leds, hwmon, input etc. for any userspace application to access.

True, but:

> Furthermore it is required for proper shutdown and reboot, which is a
> kernel task.

What do you mean by this?  What does it do for shutdown and reboot?

Is there an out-of-tree C kernel driver for this somewhere?  Or does it
all just work through userspace today on these devices?

> On arm devices, it completely takes care of the poweroff and reboot.
> There is already a driver here drivers/power/reset/qnap-poweroff.c,
> which seems to be primarily developed for QNAP, but works for Synology
> too.

But that's not this device, that's a different device and driver.

> On x86 devices, is uses ACPI Sleep, but must still announce the
> poweroff / reboot prior to the firmware call to the device for proper
> shutdown / reboot. There is no existing driver that takes care of this
> yet.

Then that should be a kernel driver, no need for the led blinks to be a
kernel driver if they don't have to, right?  We try to only put stuff in
the kernel that _has_ to be in the kernel, within reason.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-09 12:52         ` Markus Probst
  2026-03-09 13:07           ` Greg Kroah-Hartman
@ 2026-03-09 13:32           ` Danilo Krummrich
  2026-03-09 13:38             ` Markus Probst
  1 sibling, 1 reply; 21+ messages in thread
From: Danilo Krummrich @ 2026-03-09 13:32 UTC (permalink / raw)
  To: Markus Probst, Greg Kroah-Hartman
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	devicetree, linux-kernel, rust-for-linux

On Mon Mar 9, 2026 at 1:52 PM CET, Markus Probst wrote:
> Yes. I will split it into multiple drivers using the aux bus in the
> next revision.

Independent of the other discussion whether this belongs into the kernel in the
first place, reading over the cover letter and commit message I understood the
following.

  "Synology uses a microcontroller in their NAS devices connected to a serial
  port [...]" controlling LEDs, fan speeds, a beeper, etc.

  I.e. it muliplexes several physical functions that belong to different
  subsystems, such as hwmon, input, etc. over a single serial port.

This sounds like a textbook candidate for MFD to me.

I.e. there is a very loose coupling of the different functions that make up for
entirely independent drivers, except that they share the same serial port
connection.

Whereas the auxiliary bus is more for very complicated devices to be broken down
into more managable (sometimes optional) sub-domains, where the corresponding
drivers usually have driver specific APIs to interact with each other.

- Danilo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-09 13:07           ` Greg Kroah-Hartman
@ 2026-03-09 13:34             ` Markus Probst
  0 siblings, 0 replies; 21+ messages in thread
From: Markus Probst @ 2026-03-09 13:34 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, devicetree, linux-kernel, rust-for-linux

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

On Mon, 2026-03-09 at 14:07 +0100, Greg Kroah-Hartman wrote:
> On Mon, Mar 09, 2026 at 12:52:26PM +0000, Markus Probst wrote:
> > On Mon, 2026-03-09 at 06:56 +0100, Greg Kroah-Hartman wrote:
> > > On Sun, Mar 08, 2026 at 07:15:16PM +0000, Markus Probst wrote:
> > > > On Sun, 2026-03-08 at 19:55 +0100, Greg Kroah-Hartman wrote:
> > > > > On Sun, Mar 08, 2026 at 06:41:20PM +0000, 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.
> > > > > 
> > > > > Why is this a mfd device?  Shouldn't it be an aux device?
> > > > > 
> > > > > But this is just a serial port connection, so why is a kernel driver
> > > > > needed at all?
> > > > I am not sure what you mean.
> > > 
> > > Can't this just be controlled from userspace over the tty device to the
> > > uart this device uses?  Why is a kernel driver needed at all?
> > Like with any other bus device, it can be controlled by userspace.
> 
> Great, then usually that means it should not be a kernel driver :)
Not sure if this is an argument. The same would apply to the majority
of kernel drivers.
> 
> > But the kernel already provides the necessary userspace interfaces for
> > leds, hwmon, input etc. for any userspace application to access.
> 
> True, but:
> 
> > Furthermore it is required for proper shutdown and reboot, which is a
> > kernel task.
> 
> What do you mean by this?  What does it do for shutdown and reboot?
> 
> Is there an out-of-tree C kernel driver for this somewhere?  Or does it
> all just work through userspace today on these devices?
On the proprietary os of those devices,
the shutdown and reboot part is done in their modified version of the
4.4.x linux kernel.

Most of the interactions with this device however is in a proprietary
kernel module called "synobios" (source code not available), which
exposes this via their own proprietary ioctls.

> 
> > On arm devices, it completely takes care of the poweroff and reboot.
> > There is already a driver here drivers/power/reset/qnap-poweroff.c,
> > which seems to be primarily developed for QNAP, but works for Synology
> > too.
> 
> But that's not this device, that's a different device and driver.
Different driver, but supports the same device. See the "Can also be
used on Synology devices." comment on top and the "synology,power-off"
entry in the of_match_table.

> 
> > On x86 devices, is uses ACPI Sleep, but must still announce the
> > poweroff / reboot prior to the firmware call to the device for proper
> > shutdown / reboot. There is no existing driver that takes care of this
> > yet.
> 
> Then that should be a kernel driver, no need for the led blinks to be a
> kernel driver if they don't have to, right?  We try to only put stuff in
> the kernel that _has_ to be in the kernel, within reason.
Are you trying to refer to the led part of this driver, or to a prior
patch series by me regarding a more feature-rich disk trigger for led
blinking ("leds: extend disk trigger") ?

In case of the later, I have started working on a non-device-specific
userspace daemon as replacement.

> 
> thanks,
> 
> greg k-h

Thanks
- Markus Probst

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-09 13:32           ` Danilo Krummrich
@ 2026-03-09 13:38             ` Markus Probst
  2026-03-09 15:15               ` Lee Jones
  0 siblings, 1 reply; 21+ messages in thread
From: Markus Probst @ 2026-03-09 13:38 UTC (permalink / raw)
  To: Danilo Krummrich, Greg Kroah-Hartman
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	devicetree, linux-kernel, rust-for-linux

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

On Mon, 2026-03-09 at 14:32 +0100, Danilo Krummrich wrote:
> On Mon Mar 9, 2026 at 1:52 PM CET, Markus Probst wrote:
> > Yes. I will split it into multiple drivers using the aux bus in the
> > next revision.
> 
> Independent of the other discussion whether this belongs into the kernel in the
> first place, reading over the cover letter and commit message I understood the
> following.
> 
>   "Synology uses a microcontroller in their NAS devices connected to a serial
>   port [...]" controlling LEDs, fan speeds, a beeper, etc.
> 
>   I.e. it muliplexes several physical functions that belong to different
>   subsystems, such as hwmon, input, etc. over a single serial port.
> 
> This sounds like a textbook candidate for MFD to me.
> 
> I.e. there is a very loose coupling of the different functions that make up for
> entirely independent drivers, except that they share the same serial port
> connection.
> 
> Whereas the auxiliary bus is more for very complicated devices to be broken down
> into more managable (sometimes optional) sub-domains, where the corresponding
> drivers usually have driver specific APIs to interact with each other.
> 
> - Danilo

QNAP and Synology do things very similarly.
There is already a driver for QNAP devices:

drivers/mfd/qnap-mcu.c
drivers/leds/leds-qnap-mcu.c
drivers/input/misc/qnap-mcu-input.c
drivers/hwmon/qnap-mcu-hwmon.c
drivers/nvmem/qnap-mcu-eeprom.c

drivers/power/reset/qnap-poweroff.c (this one is not part of the mfd)

and I try to implement the equivalent for Synology devices.
Given its a MFD I would assume the same applies to this driver?

Thanks
- Markus Probst


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-09 13:38             ` Markus Probst
@ 2026-03-09 15:15               ` Lee Jones
  2026-03-09 15:20                 ` Markus Probst
  2026-03-09 15:37                 ` Danilo Krummrich
  0 siblings, 2 replies; 21+ messages in thread
From: Lee Jones @ 2026-03-09 15:15 UTC (permalink / raw)
  To: Markus Probst
  Cc: Danilo Krummrich, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Miguel Ojeda, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, devicetree, linux-kernel,
	rust-for-linux

On Mon, 09 Mar 2026, Markus Probst wrote:

> On Mon, 2026-03-09 at 14:32 +0100, Danilo Krummrich wrote:
> > On Mon Mar 9, 2026 at 1:52 PM CET, Markus Probst wrote:
> > > Yes. I will split it into multiple drivers using the aux bus in the
> > > next revision.
> > 
> > Independent of the other discussion whether this belongs into the kernel in the
> > first place, reading over the cover letter and commit message I understood the
> > following.
> > 
> >   "Synology uses a microcontroller in their NAS devices connected to a serial
> >   port [...]" controlling LEDs, fan speeds, a beeper, etc.
> > 
> >   I.e. it muliplexes several physical functions that belong to different
> >   subsystems, such as hwmon, input, etc. over a single serial port.
> > 
> > This sounds like a textbook candidate for MFD to me.

Then you do not know what a textbook candidate for MFD is. :)

What part of the MFD API does this device utilise?

> > I.e. there is a very loose coupling of the different functions that make up for
> > entirely independent drivers, except that they share the same serial port
> > connection.
> > 
> > Whereas the auxiliary bus is more for very complicated devices to be broken down
> > into more managable (sometimes optional) sub-domains, where the corresponding
> > drivers usually have driver specific APIs to interact with each other.
> > 
> > - Danilo
> 
> QNAP and Synology do things very similarly.
> There is already a driver for QNAP devices:
> 
> drivers/mfd/qnap-mcu.c
> drivers/leds/leds-qnap-mcu.c
> drivers/input/misc/qnap-mcu-input.c
> drivers/hwmon/qnap-mcu-hwmon.c
> drivers/nvmem/qnap-mcu-eeprom.c
> 
> drivers/power/reset/qnap-poweroff.c (this one is not part of the mfd)
> 
> and I try to implement the equivalent for Synology devices.
> Given its a MFD I would assume the same applies to this driver?

Of course not.

The QNAP driver above calls devm_mfd_add_devices().

This one uses none of the MFD functionality provided.

Linux supports 10's if not 100's of devices which do more than one
thing.  Only a fraction of them are Linux MFDs.  MFD in Linux is an API,
not a type of device.

Don't get me wrong, you could probably code up this device as a Linux
MFD, but you have chosen not to, so therefore it cannot reside here.

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-09 15:15               ` Lee Jones
@ 2026-03-09 15:20                 ` Markus Probst
  2026-03-09 15:27                   ` Lee Jones
  2026-03-09 15:37                 ` Danilo Krummrich
  1 sibling, 1 reply; 21+ messages in thread
From: Markus Probst @ 2026-03-09 15:20 UTC (permalink / raw)
  To: Lee Jones
  Cc: Danilo Krummrich, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Miguel Ojeda, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, devicetree, linux-kernel,
	rust-for-linux

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

On Mon, 2026-03-09 at 15:15 +0000, Lee Jones wrote:
> On Mon, 09 Mar 2026, Markus Probst wrote:
> 
> > On Mon, 2026-03-09 at 14:32 +0100, Danilo Krummrich wrote:
> > > On Mon Mar 9, 2026 at 1:52 PM CET, Markus Probst wrote:
> > > > Yes. I will split it into multiple drivers using the aux bus in the
> > > > next revision.
> > > 
> > > Independent of the other discussion whether this belongs into the kernel in the
> > > first place, reading over the cover letter and commit message I understood the
> > > following.
> > > 
> > >   "Synology uses a microcontroller in their NAS devices connected to a serial
> > >   port [...]" controlling LEDs, fan speeds, a beeper, etc.
> > > 
> > >   I.e. it muliplexes several physical functions that belong to different
> > >   subsystems, such as hwmon, input, etc. over a single serial port.
> > > 
> > > This sounds like a textbook candidate for MFD to me.
> 
> Then you do not know what a textbook candidate for MFD is. :)
> 
> What part of the MFD API does this device utilise?
> 
> > > I.e. there is a very loose coupling of the different functions that make up for
> > > entirely independent drivers, except that they share the same serial port
> > > connection.
> > > 
> > > Whereas the auxiliary bus is more for very complicated devices to be broken down
> > > into more managable (sometimes optional) sub-domains, where the corresponding
> > > drivers usually have driver specific APIs to interact with each other.
> > > 
> > > - Danilo
> > 
> > QNAP and Synology do things very similarly.
> > There is already a driver for QNAP devices:
> > 
> > drivers/mfd/qnap-mcu.c
> > drivers/leds/leds-qnap-mcu.c
> > drivers/input/misc/qnap-mcu-input.c
> > drivers/hwmon/qnap-mcu-hwmon.c
> > drivers/nvmem/qnap-mcu-eeprom.c
> > 
> > drivers/power/reset/qnap-poweroff.c (this one is not part of the mfd)
> > 
> > and I try to implement the equivalent for Synology devices.
> > Given its a MFD I would assume the same applies to this driver?
> 
> Of course not.
> 
> The QNAP driver above calls devm_mfd_add_devices().
> 
> This one uses none of the MFD functionality provided.
> 
> Linux supports 10's if not 100's of devices which do more than one
> thing.  Only a fraction of them are Linux MFDs.  MFD in Linux is an API,
> not a type of device.
> 
> Don't get me wrong, you could probably code up this device as a Linux
> MFD, but you have chosen not to, so therefore it cannot reside here.
From what I have read so far, the way its written it probably can't
live anywhere inside the kernel right now. So it will be restructured
to either use mfd or auxiliary, which both involve splitting the driver
up into multiple pieces. From what Danilo is writing, it will likely be
mfd.

Thanks
- Markus Probst


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-09 15:20                 ` Markus Probst
@ 2026-03-09 15:27                   ` Lee Jones
  0 siblings, 0 replies; 21+ messages in thread
From: Lee Jones @ 2026-03-09 15:27 UTC (permalink / raw)
  To: Markus Probst
  Cc: Danilo Krummrich, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Miguel Ojeda, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, devicetree, linux-kernel,
	rust-for-linux

On Mon, 09 Mar 2026, Markus Probst wrote:

> On Mon, 2026-03-09 at 15:15 +0000, Lee Jones wrote:
> > On Mon, 09 Mar 2026, Markus Probst wrote:
> > 
> > > On Mon, 2026-03-09 at 14:32 +0100, Danilo Krummrich wrote:
> > > > On Mon Mar 9, 2026 at 1:52 PM CET, Markus Probst wrote:
> > > > > Yes. I will split it into multiple drivers using the aux bus in the
> > > > > next revision.
> > > > 
> > > > Independent of the other discussion whether this belongs into the kernel in the
> > > > first place, reading over the cover letter and commit message I understood the
> > > > following.
> > > > 
> > > >   "Synology uses a microcontroller in their NAS devices connected to a serial
> > > >   port [...]" controlling LEDs, fan speeds, a beeper, etc.
> > > > 
> > > >   I.e. it muliplexes several physical functions that belong to different
> > > >   subsystems, such as hwmon, input, etc. over a single serial port.
> > > > 
> > > > This sounds like a textbook candidate for MFD to me.
> > 
> > Then you do not know what a textbook candidate for MFD is. :)
> > 
> > What part of the MFD API does this device utilise?
> > 
> > > > I.e. there is a very loose coupling of the different functions that make up for
> > > > entirely independent drivers, except that they share the same serial port
> > > > connection.
> > > > 
> > > > Whereas the auxiliary bus is more for very complicated devices to be broken down
> > > > into more managable (sometimes optional) sub-domains, where the corresponding
> > > > drivers usually have driver specific APIs to interact with each other.
> > > > 
> > > > - Danilo
> > > 
> > > QNAP and Synology do things very similarly.
> > > There is already a driver for QNAP devices:
> > > 
> > > drivers/mfd/qnap-mcu.c
> > > drivers/leds/leds-qnap-mcu.c
> > > drivers/input/misc/qnap-mcu-input.c
> > > drivers/hwmon/qnap-mcu-hwmon.c
> > > drivers/nvmem/qnap-mcu-eeprom.c
> > > 
> > > drivers/power/reset/qnap-poweroff.c (this one is not part of the mfd)
> > > 
> > > and I try to implement the equivalent for Synology devices.
> > > Given its a MFD I would assume the same applies to this driver?
> > 
> > Of course not.
> > 
> > The QNAP driver above calls devm_mfd_add_devices().
> > 
> > This one uses none of the MFD functionality provided.
> > 
> > Linux supports 10's if not 100's of devices which do more than one
> > thing.  Only a fraction of them are Linux MFDs.  MFD in Linux is an API,
> > not a type of device.
> > 
> > Don't get me wrong, you could probably code up this device as a Linux
> > MFD, but you have chosen not to, so therefore it cannot reside here.
> From what I have read so far, the way its written it probably can't
> live anywhere inside the kernel right now.

There is a place for everything in the kernel.

> So it will be restructured
> to either use mfd or auxiliary, which both involve splitting the driver
> up into multiple pieces.

The driver should absolutely be split up into multiple pieces with each
part being redistributed into its associated subsystem.

> From what Danilo is writing, it will likely be
> mfd.

If you use the MFD API, then it can live in drivers/mfd.  If you go with
the Auxiliary route, then you'll have to find somewhere else to put it.

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-09 15:15               ` Lee Jones
  2026-03-09 15:20                 ` Markus Probst
@ 2026-03-09 15:37                 ` Danilo Krummrich
  2026-03-09 15:50                   ` Lee Jones
  1 sibling, 1 reply; 21+ messages in thread
From: Danilo Krummrich @ 2026-03-09 15:37 UTC (permalink / raw)
  To: Lee Jones
  Cc: Markus Probst, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Miguel Ojeda, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, devicetree, linux-kernel,
	rust-for-linux

On Mon Mar 9, 2026 at 4:15 PM CET, Lee Jones wrote:
> On Mon, 09 Mar 2026, Markus Probst wrote:
>
>> On Mon, 2026-03-09 at 14:32 +0100, Danilo Krummrich wrote:
>> > On Mon Mar 9, 2026 at 1:52 PM CET, Markus Probst wrote:
>> > > Yes. I will split it into multiple drivers using the aux bus in the
>> > > next revision.
>> > 
>> > Independent of the other discussion whether this belongs into the kernel in the
>> > first place, reading over the cover letter and commit message I understood the
>> > following.
>> > 
>> >   "Synology uses a microcontroller in their NAS devices connected to a serial
>> >   port [...]" controlling LEDs, fan speeds, a beeper, etc.
>> > 
>> >   I.e. it muliplexes several physical functions that belong to different
>> >   subsystems, such as hwmon, input, etc. over a single serial port.
>> > 
>> > This sounds like a textbook candidate for MFD to me.
>
> Then you do not know what a textbook candidate for MFD is. :)
>
> What part of the MFD API does this device utilise?

I did *not* say this driver - as in the code that was posted - is a textbook
example for MFD.

I said that the hardware that is supposed to be fully supported eventually is a
textbook candidate for an MFD driver, i.e. it should use things like
devm_mfd_add_devices().

I think this was obvious from what I wrote above.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
  2026-03-09 15:37                 ` Danilo Krummrich
@ 2026-03-09 15:50                   ` Lee Jones
  0 siblings, 0 replies; 21+ messages in thread
From: Lee Jones @ 2026-03-09 15:50 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Markus Probst, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Miguel Ojeda, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, devicetree, linux-kernel,
	rust-for-linux

On Mon, 09 Mar 2026, Danilo Krummrich wrote:

> On Mon Mar 9, 2026 at 4:15 PM CET, Lee Jones wrote:
> > On Mon, 09 Mar 2026, Markus Probst wrote:
> >
> >> On Mon, 2026-03-09 at 14:32 +0100, Danilo Krummrich wrote:
> >> > On Mon Mar 9, 2026 at 1:52 PM CET, Markus Probst wrote:
> >> > > Yes. I will split it into multiple drivers using the aux bus in the
> >> > > next revision.
> >> > 
> >> > Independent of the other discussion whether this belongs into the kernel in the
> >> > first place, reading over the cover letter and commit message I understood the
> >> > following.
> >> > 
> >> >   "Synology uses a microcontroller in their NAS devices connected to a serial
> >> >   port [...]" controlling LEDs, fan speeds, a beeper, etc.
> >> > 
> >> >   I.e. it muliplexes several physical functions that belong to different
> >> >   subsystems, such as hwmon, input, etc. over a single serial port.
> >> > 
> >> > This sounds like a textbook candidate for MFD to me.
> >
> > Then you do not know what a textbook candidate for MFD is. :)
> >
> > What part of the MFD API does this device utilise?
> 
> I did *not* say this driver - as in the code that was posted - is a textbook
> example for MFD.
> 
> I said that the hardware that is supposed to be fully supported eventually is a
> textbook candidate for an MFD driver, i.e. it should use things like
> devm_mfd_add_devices().
> 
> I think this was obvious from what I wrote above.

No, not at all.  Or there would be no confusion. :)

But yes, I agree, if re-authored this could fit into MFD.

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2026-03-09 15:50 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-08 18:41 [PATCH v2 0/2] Introduce Synology Microp driver Markus Probst
2026-03-08 18:41 ` [PATCH v2 1/2] dt-bindings: mfd: Add synology,microp device Markus Probst
2026-03-09  7:17   ` Krzysztof Kozlowski
2026-03-08 18:41 ` [PATCH v2 2/2] mfd: Add initial synology microp driver Markus Probst
2026-03-08 18:55   ` Greg Kroah-Hartman
2026-03-08 19:15     ` Markus Probst
2026-03-09  5:56       ` Greg Kroah-Hartman
2026-03-09  9:43         ` Lee Jones
2026-03-09 12:52         ` Markus Probst
2026-03-09 13:07           ` Greg Kroah-Hartman
2026-03-09 13:34             ` Markus Probst
2026-03-09 13:32           ` Danilo Krummrich
2026-03-09 13:38             ` Markus Probst
2026-03-09 15:15               ` Lee Jones
2026-03-09 15:20                 ` Markus Probst
2026-03-09 15:27                   ` Lee Jones
2026-03-09 15:37                 ` Danilo Krummrich
2026-03-09 15:50                   ` Lee Jones
2026-03-08 18:56   ` Greg Kroah-Hartman
2026-03-08 19:23     ` Markus Probst
2026-03-09  5:55       ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox