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

[Greg Kroah-Hartman]

On Mon, Mar 16, 2026 at 01:43:44PM +0000, Markus Probst wrote:
> 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.

So that means a write of a set of bytes to the serial port will cause
the machine to reboot or shutdown?

> 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

For "input" what exactly does the input device show up as?  A power
button?  Something else?

For hwmon, that makes sense to have a kernel driver.

For leds, that depends on what you want to do with the led, as in the
end you are just controlling it from userspace anyway :)

> 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.

Yes, I kind of know what drivers and classes do and why they are needed,
that's not the point here. :)

> 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.

Ok great, then make a single driver that handles all of this, like other
drivers/platform/ drivers do today, and all should be fine.

thanks,

greg k-h