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

From: Markus Probst

Date: Tue Apr 21 2026 - 10:19:38 EST

On Tue, 2026-04-21 at 14:59 +0300, Ilpo Järvinen wrote:
> On Mon, 20 Apr 2026, Markus Probst wrote:
>
> > Add a initial synology microp driver, written in Rust.
> > The driver targets a microcontroller found in Synology NAS devices. It
> > currently only supports controlling of the power led, status led, alert
> > led and usb led. Other components such as fan control or handling
> > on-device buttons will be added once the required rust abstractions are
> > there.
> >
> > This driver can be used both on arm and x86, thus it goes into the root
> > directory of drivers/platform.
> >
> > Tested successfully on a Synology DS923+.
> >
> > Signed-off-by: Markus Probst <markus.probst@xxxxxxxxx>
> > ---
> > MAINTAINERS | 6 +
> > drivers/platform/Kconfig | 2 +
> > drivers/platform/Makefile | 1 +
> > drivers/platform/synology_microp/Kconfig | 13 +
> > drivers/platform/synology_microp/Makefile | 3 +
> > drivers/platform/synology_microp/TODO | 7 +
> > drivers/platform/synology_microp/command.rs | 54 ++++
> > drivers/platform/synology_microp/led.rs | 281 +++++++++++++++++++++
> > drivers/platform/synology_microp/model.rs | 49 ++++
> > .../platform/synology_microp/synology_microp.rs | 110 ++++++++
> > 10 files changed, 526 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index c1c686846cdd..49f08290eed0 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -25555,6 +25555,12 @@ F: drivers/dma-buf/sync_*
> > F: include/linux/sync_file.h
> > F: include/uapi/linux/sync_file.h
> >
> > +SYNOLOGY MICROP DRIVER
> > +M: Markus Probst <markus.probst@xxxxxxxxx>
>
> You should probably add:
>
> L: platform-driver-x86@xxxxxxxxxxxxxxx
>
> Through which tree the patches to this driver are generally expected to be
> picked up?
I suppose platform-drivers-x86. The driver itself can be used both on
x86 and arm64. Although I also have seen Synology devices with PowerPC
(no device with PowerPC is supported in the driver yet).

>
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/embedded-controller/synology,ds1825p-microp.yaml
> > +F: drivers/platform/synology_microp/
> > +
> > SYNOPSYS ARC ARCHITECTURE
> > M: Vineet Gupta <vgupta@xxxxxxxxxx>
> > L: linux-snps-arc@xxxxxxxxxxxxxxxxxxx
> > diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
> > index 312788f249c9..996050566a4a 100644
> > --- a/drivers/platform/Kconfig
> > +++ b/drivers/platform/Kconfig
> > @@ -22,3 +22,5 @@ source "drivers/platform/arm64/Kconfig"
> > source "drivers/platform/raspberrypi/Kconfig"
> >
> > source "drivers/platform/wmi/Kconfig"
> > +
> > +source "drivers/platform/synology_microp/Kconfig"
> > diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
> > index fa322e7f8716..2381872e9133 100644
> > --- a/drivers/platform/Makefile
> > +++ b/drivers/platform/Makefile
> > @@ -15,3 +15,4 @@ obj-$(CONFIG_SURFACE_PLATFORMS) += surface/
> > obj-$(CONFIG_ARM64_PLATFORM_DEVICES) += arm64/
> > obj-$(CONFIG_BCM2835_VCHIQ) += raspberrypi/
> > obj-$(CONFIG_ACPI_WMI) += wmi/
> > +obj-$(CONFIG_SYNOLOGY_MICROP) += synology_microp/
> > diff --git a/drivers/platform/synology_microp/Kconfig b/drivers/platform/synology_microp/Kconfig
> > new file mode 100644
> > index 000000000000..7c4d8f2808f0
> > --- /dev/null
> > +++ b/drivers/platform/synology_microp/Kconfig
> > @@ -0,0 +1,13 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +config SYNOLOGY_MICROP
> > + tristate "Synology Microp driver"
> > + depends on LEDS_CLASS && LEDS_CLASS_MULTICOLOR
> > + depends on RUST_SERIAL_DEV_BUS_ABSTRACTIONS
> > + help
> > + Enable support for the MCU found in Synology NAS devices.
> > +
> > + This is needed to properly shutdown and reboot the device, as well as
> > + additional functionality like fan and LED control.
> > +
> > + This driver is work in progress and may not be fully functional.
> > diff --git a/drivers/platform/synology_microp/Makefile b/drivers/platform/synology_microp/Makefile
> > new file mode 100644
> > index 000000000000..63585ccf76e4
> > --- /dev/null
> > +++ b/drivers/platform/synology_microp/Makefile
> > @@ -0,0 +1,3 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +obj-y += synology_microp.o
> > diff --git a/drivers/platform/synology_microp/TODO b/drivers/platform/synology_microp/TODO
> > new file mode 100644
> > index 000000000000..1961a33115db
> > --- /dev/null
> > +++ b/drivers/platform/synology_microp/TODO
> > @@ -0,0 +1,7 @@
> > +TODO:
> > +- add missing components:
> > + - handle on-device buttons (Power, Factory reset, "USB Copy")
> > + - handle fan failure
> > + - beeper
> > + - fan speed control
> > + - correctly perform device power-off and restart on Synology devices
>
> Is this TODO list really needed within the kernel distribution?
Not really. Although it indicates the current state of the driver.

>
> If you planning on add these features (relatively) soon yourself (perhaps
> depending on when the rust infra required for these features becomes
> available), the list would not be that useful for other developers at all.
Yes. Also I haven't seen anyone work on input, hwmon, reboot/sysoff
rust abstractions yet, so I will likely need to add those as well.

>
> > diff --git a/drivers/platform/synology_microp/command.rs b/drivers/platform/synology_microp/command.rs
> > new file mode 100644
> > index 000000000000..430cb858e1c3
> > --- /dev/null
> > +++ b/drivers/platform/synology_microp/command.rs
> > @@ -0,0 +1,54 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +use kernel::{
> > + device::Bound,
> > + error::Result,
> > + serdev, //
> > +};
> > +
> > +use crate::led;
> > +
> > +#[expect(
> > + clippy::enum_variant_names,
> > + reason = "future variants will not end with Led"
> > +)]
> > +pub(crate) enum Command {
> > + PowerLed(led::State),
> > + StatusLed(led::StatusLedColor, led::State),
> > + AlertLed(led::State),
> > + UsbLed(led::State),
> > + EsataLed(led::State),
> > +}
> > +
> > +impl Command {
> > + pub(crate) fn write(self, dev: &serdev::Device<Bound>) -> Result {
> > + dev.write_all(
> > + match self {
> > + Self::PowerLed(led::State::On) => &[0x34],
> > + Self::PowerLed(led::State::Blink) => &[0x35],
> > + Self::PowerLed(led::State::Off) => &[0x36],
> > +
> > + Self::StatusLed(_, led::State::Off) => &[0x37],
> > + Self::StatusLed(led::StatusLedColor::Green, led::State::On) => &[0x38],
> > + Self::StatusLed(led::StatusLedColor::Green, led::State::Blink) => &[0x39],
> > + Self::StatusLed(led::StatusLedColor::Orange, led::State::On) => &[0x3A],
> > + Self::StatusLed(led::StatusLedColor::Orange, led::State::Blink) => &[0x3B],
> > +
> > + Self::AlertLed(led::State::On) => &[0x4C, 0x41, 0x31],
> > + Self::AlertLed(led::State::Blink) => &[0x4C, 0x41, 0x32],
> > + Self::AlertLed(led::State::Off) => &[0x4C, 0x41, 0x33],
> > +
> > + Self::UsbLed(led::State::On) => &[0x40],
> > + Self::UsbLed(led::State::Blink) => &[0x41],
> > + Self::UsbLed(led::State::Off) => &[0x42],
> > +
> > + Self::EsataLed(led::State::On) => &[0x4C, 0x45, 0x31],
> > + Self::EsataLed(led::State::Blink) => &[0x4C, 0x45, 0x32],
> > + Self::EsataLed(led::State::Off) => &[0x4C, 0x45, 0x33],
> > + },
> > + serdev::Timeout::Max,
> > + )?;
> > + dev.wait_until_sent(serdev::Timeout::Max);
> > + Ok(())
> > + }
> > +}
> > diff --git a/drivers/platform/synology_microp/led.rs b/drivers/platform/synology_microp/led.rs
> > new file mode 100644
> > index 000000000000..f89998a7e6b4
> > --- /dev/null
> > +++ b/drivers/platform/synology_microp/led.rs
> > @@ -0,0 +1,281 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +use kernel::{
> > + device::Bound,
> > + devres::{
> > + self,
> > + Devres, //
> > + },
> > + led::{
> > + self,
> > + LedOps,
> > + MultiColorSubLed, //
> > + },
> > + new_mutex,
> > + prelude::*,
> > + serdev,
> > + str::CString,
> > + sync::Mutex, //
> > +};
> > +use pin_init::pin_init_scope;
> > +
> > +use crate::{
> > + command::Command,
> > + model::Model, //
> > +};
> > +
> > +#[pin_data]
> > +pub(crate) struct Data {
> > + #[pin]
> > + status: Devres<led::MultiColorDevice<StatusLedHandler>>,
> > + power_name: CString,
> > + #[pin]
> > + power: Devres<led::Device<LedHandler>>,
> > +}
> > +
> > +impl Data {
> > + pub(super) fn register<'a>(
> > + dev: &'a serdev::Device<Bound>,
> > + model: &'a Model,
> > + ) -> impl PinInit<Self, Error> + 'a {
> > + pin_init_scope(move || {
> > + if let Some(color) = model.led_alert {
> > + let name = CString::try_from_fmt(fmt!("{}:alarm", color.as_c_str().to_str()?))?;
> > + devres::register(
> > + dev.as_ref(),
> > + led::DeviceBuilder::new().color(color).name(&name).build(
> > + dev,
> > + try_pin_init!(LedHandler {
> > + blink <- new_mutex!(false),
> > + command: Command::AlertLed,
> > + }),
> > + ),
> > + GFP_KERNEL,
> > + )?;
> > + }
> > +
> > + if model.led_usb_copy {
> > + devres::register(
> > + dev.as_ref(),
> > + led::DeviceBuilder::new()
> > + .color(led::Color::Green)
> > + .name(c"green:usb")
> > + .build(
> > + dev,
> > + try_pin_init!(LedHandler {
> > + blink <- new_mutex!(false),
> > + command: Command::UsbLed,
> > + }),
> > + ),
> > + GFP_KERNEL,
> > + )?;
> > + }
> > +
> > + if model.led_esata {
> > + devres::register(
> > + dev.as_ref(),
> > + led::DeviceBuilder::new()
> > + .color(led::Color::Green)
> > + .name(c"green:esata")
> > + .build(
> > + dev,
> > + try_pin_init!(LedHandler {
> > + blink <- new_mutex!(false),
> > + command: Command::EsataLed,
> > + }),
> > + ),
> > + GFP_KERNEL,
> > + )?;
> > + }
> > +
> > + Ok(try_pin_init!(Self {
> > + status <- led::DeviceBuilder::new()
> > + .color(led::Color::Multi)
> > + .name(c"multicolor:status")
> > + .build_multicolor(
> > + dev,
> > + try_pin_init!(StatusLedHandler {
> > + blink <- new_mutex!(false),
> > + }),
> > + StatusLedHandler::SUBLEDS,
> > + ),
> > + power_name: CString::try_from_fmt(fmt!(
> > + "{}:power",
> > + model.led_power.as_c_str().to_str()?
> > + ))?,
> > + power <- led::DeviceBuilder::new()
> > + .color(model.led_power)
> > + .name(power_name)
> > + .build(
> > + dev,
> > + try_pin_init!(LedHandler {
> > + blink <- new_mutex!(true),
> > + command: Command::PowerLed,
> > + }),
> > + ),
> > + }))
> > + })
> > + }
> > +}
> > +
> > +#[derive(Copy, Clone)]
> > +pub(crate) enum StatusLedColor {
> > + Green,
> > + Orange,
> > +}
> > +
> > +#[derive(Copy, Clone)]
> > +pub(crate) enum State {
> > + On,
> > + Blink,
> > + Off,
> > +}
> > +
> > +#[pin_data]
> > +struct LedHandler {
> > + #[pin]
> > + blink: Mutex<bool>,
> > + command: fn(State) -> Command,
> > +}
> > +
> > +/// Blink delay measured using video recording on DS923+ for Power and Status Led.
> > +///
> > +/// We assume it is the same for all other leds and models.
> > +const BLINK_DELAY: usize = 167;
>
> On C side time related consts are required to include the unit in their
> name. Perhaps Rust code should also follow this convention?
How about `const BLINK_DELAY: Msecs` ? The unit would be implied
through the already existing type alias `kernel::time::Msecs` for u32.

Thanks
- Markus Probst

Attachment: signature.asc
Description: This is a digitally signed message part