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

[Markus Probst <markus.probst@posteo.de>]

[-- 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 --]