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

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

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

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


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