Re: [PATCH v3 7/7] leds: add synology microp led driver

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

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

On Mon, 2026-03-16 at 07:33 +0100, Greg Kroah-Hartman wrote:
> On Sun, Mar 15, 2026 at 08:41:06PM +0100, Danilo Krummrich wrote:
> > I.e. if we can't (easily) use mfd cells and would need a custom API, then why
> > even split it up at all, given that splitting it up would probably the most
> > complicated part of the whole driver.
> > 
> > Greg, what do you think?
> 
> I think this has yet to be proven to be a kernel driver at all at this
> point, and not just a userspace daemon that listens to the serial port
> and then does what is needed from there :)
> 
> Or, if someone can prove that the operations on this serial data stream
> actually do require it to be in the kernel (which I have yet to see a
> list of what this connection does, did I miss it?) then a single driver,
> under the drivers/platform section of the kernel tree makes sense.
The sysoff component is strictly necessary for poweroff and reboot.

On ARM64 Synology NAS devices it is needed so the device actually
powers off after calling
`syscall(SYS_reboot, LINUX_REBOOT_MAGIC1, LINUX_REBOOT_MAGIC2,
LINUX_REBOOT_CMD_POWER_OFF, NULL);`
. Otherwise it would stay on.
Same applies to reboot.

On x86 it isn't clearly documented what sending the poweroff and reboot
command to the microp device exactly does, so this is based on
observations. It should be sent before issuing a poweroff or reboot via
ACPI Sleep. On reboot it resets various device states, so fan speeds go
to default, leds turn off etc., so it behaves like a coldboot.
On poweroff it will mark it as graceful shutdown (i. e. the device
won't turn automatically on, because it thinks a power-loss happend).

For the other components:
- leds
- hwmon
- input

It could theoratically be implemented in userspace. A userspace daemon
could theoratically control the fan speeds directly, issue a systemd
shutdown on power button press, control the leds directly etc.

But honestly, I don't understand why this is an argument.
With that argument drivers/leds, drivers/hwmon and drivers/input would
not even exist, because everything could be implemented in userspace
via
- I2C: /dev/i2c-* (drivers/i2c/i2c-dev.c)
- MMIO: /dev/ioport and /dev/mem (drivers/char/mem.c)
- GPIO: /sys/class/gpio (drivers/gpio/gpiolib-sysfs.c)
- SPI: /dev/spidev* (drivers/spi/spidev.c)
- PCI: /sys/class/pci_bus/ (drivers/pci/pci-sysfs.c)
- Serial: /dev/ttyS*
and likely almost any other bus device too.

Generally speaking, the kernel and its drivers is the layer between
hardware and software. It provides the hardware abstractions as
userspace interfaces. So any software on the same cpu architecture can
work with any hardware, as long as there is a kernel driver.

In the case of this driver, it means
- *any* led daemon can control the leds
- *any* fan control daemon can control the fan speed and frequency
- *any* monitoring software can view the provided sensors
- *any* init system can react to the power button
- *any* process can request a reboot or shutdown
.
I think this is the expected behaviour.

Thanks
- Markus Probst

> 
> thanks,
> 
> greg k-h

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