[PATCH v2 2/2] mfd: Add initial synology microp driver

Markus Probst posted 2 patches 2 days, 4 hours ago
[PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Markus Probst 2 days, 4 hours ago
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
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Greg Kroah-Hartman 2 days, 3 hours ago
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
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Markus Probst 2 days, 3 hours ago
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
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Greg Kroah-Hartman 1 day, 16 hours ago
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
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Greg Kroah-Hartman 2 days, 3 hours ago
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
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Markus Probst 2 days, 3 hours ago
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
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Greg Kroah-Hartman 1 day, 16 hours ago
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
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Markus Probst 1 day, 9 hours ago
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
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Danilo Krummrich 1 day, 9 hours ago
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
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Markus Probst 1 day, 9 hours ago
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

Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Lee Jones 1 day, 7 hours ago
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 [李琼斯]
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Danilo Krummrich 1 day, 7 hours ago
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.
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Lee Jones 1 day, 6 hours ago
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 [李琼斯]
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Markus Probst 1 day, 7 hours ago
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

Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Lee Jones 1 day, 7 hours ago
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 [李琼斯]
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Greg Kroah-Hartman 1 day, 9 hours ago
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
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Markus Probst 1 day, 9 hours ago
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
Re: [PATCH v2 2/2] mfd: Add initial synology microp driver
Posted by Lee Jones 1 day, 12 hours ago
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 [李琼斯]