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

[Lee Jones]

On Mon, 09 Mar 2026, Markus Probst wrote:

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

There is a place for everything in the kernel.

> So it will be restructured
> to either use mfd or auxiliary, which both involve splitting the driver
> up into multiple pieces.

The driver should absolutely be split up into multiple pieces with each
part being redistributed into its associated subsystem.

> From what Danilo is writing, it will likely be
> mfd.

If you use the MFD API, then it can live in drivers/mfd.  If you go with
the Auxiliary route, then you'll have to find somewhere else to put it.

-- 
Lee Jones [李琼斯]