Re: [PATCH RFC v5 5/7] ipmi: ls2k: Relax the dependency to its mfd driver

[Corey Minyard <corey@minyard.net>]

On Tue, Aug 04, 2026 at 08:27:11PM +0800, Miao Wang wrote:
> Hi,
> 
> > 2026年8月4日 19:39，Corey Minyard <corey@minyard.net> 写道：
> > 
> > On Tue, Aug 04, 2026 at 05:40:14PM +0800, Miao Wang wrote:
> >> Hi,
> >> 
> >>> 2026年8月4日 04:46，Corey Minyard <corey@minyard.net> 写道：
> >>> 
> >>> On Tue, Aug 04, 2026 at 12:55:53AM +0800, Miao Wang via B4 Relay wrote:
> >>>> From: Miao Wang <shankerwangmiao@gmail.com>
> >>>> 
> >>>> There is no strong dependency between the IPMI driver and its mfd
> >>>> driver. Although the IPMI driver will not work without the mfd driver,
> >>>> it is not a hard dependency. The IPMI driver can actually be compiled
> >>>> without the mfd driver, and it will just fail to probe. When the mfd
> >>>> driver is loaded, the IPMI driver will probe successfully. Therefore,
> >>>> the dependency of the IPMI driver on its mfd driver should be relaxed
> >>>> to "imply" from "select". This will allow the mfd driver to be compiled
> >>>> as a module and the IPMI driver to be compiled as a part of the ipmi_si
> >>>> module. The adjustment to Kconfig for the mfd driver will be introduced
> >>>> in the later patch in this series.
> >>> 
> >>> I don't think that's what "imply" is for.  Imply seems to be for if
> >>> there is another subsystem that can use this subsystem, but doesn't
> >>> require it to exist.
> >>> 
> >>> For instance:
> >>> 
> >>> config SENSORS_NPCM7XX
> >>>       tristate "Nuvoton NPCM750 and compatible PWM and Fan controllers"
> >>>       imply THERMAL
> >>> 
> >>> The fan controller will work fine without the thermal subsystem; you
> >>> can control the fan speed without it.  But the thermal subsystem is the
> >>> logical user of this.  I looked at many of these things like this.
> >>> 
> >>> In the IPMI case, the IPMI driver is useless without the mfd part.  So
> >>> there's no point in compiling the IPMI part of this if the mfd part is
> >>> not there.
> >>> 
> >>> I could be wrong, but I can't see why you would want to do this.
> >> 
> >> The mfd part and the IPMI part loosely depend on each other. Without the
> >> IPMI part, the mfd part can still work to handle the display part.
> >> Without the mfd part, the IPMI part is indeed useless, but it will not
> >> generate compiling errors or other runtime errors. In the runtime,
> >> the IPMI part can be actually loaded earlier than the mfd part. As a
> >> result, their dependency is not that strong.
> > 
> > The operational dependency is strong, which is what I think you want to
> > convey here.
> > 
> >> 
> >> The reason why I want to change this is that "select" here requires the
> >> mfd part should also be compiled as built-in (i.e. = y), since the
> >> type of the configure entry IPMI_LS2K is bool. However, I cannot see
> >> there is no other reason preventing the mfd driver from compiling as
> >> a module. This patch series will introduce a minor fix, after which
> >> the mfd driver will be capable to be compiled as a module.
> >> 
> >> When the type of MFD_LS2K_BMC_CORE is changed to tristate, the "select"
> >> here will prevent selecting =m for MFD_LS2K_BMC_CORE. I thus believe
> >> that "select" here should be also changed.
> >> 
> >> Any suggestions on declaring the dependency of the both parts?
> > 
> > Ok, I understand now.
> > 
> > Why can't the IPMI part be compiled as a module?  Making that
> > module-capable would be the right fix, I think.  I can't see
> > why that wouldn't work.  IIRC, it was bool because the mfd part
> > was bool.
> 
> I would have agree with you if the IPMI part was a normal driver.
> However, the IPMI part is actually a part of ipmi_si. As a result,
> the IPMI part can actually be compiled as a module, but as a part
> of ipmi_si. That is why the configure entry IPMI_LS2K is bool rather
> than tristate. I don't think it is trivial to convert IPMI_LS2K from
> bool to tristate.

Yes, you are right it would be non-trivial.  It would take some redesign
of the ipmi_si module.  You are right here, probably not worth it.

> 
> > If you changed to imply, you would need a depends on the mfd core, BTW.
> 
> If imply is used here, and there is "select MFD_CORE" in MFD_LS2K_BMC_CORE,
> I think there is no need to add "depends on MFD_CORE" to IPMI_LS2K, if I
> understand it correctly.

If imply doesn't force MFD_LS2K_BMC_CORE to be selected, then it's not
going to force MFD_CORE to be selected.

> 
> I don't insist using imply here, if there is a better way to express the
> relation between both drivers.
> 
> > Also, IPMI_LS2K needs a "depends on IPMI_SI" either way.  I missed that
> > earlier.  Could you add that?
> 
> If I understand it correctly, there seems to be no need to add the
> dependency from the MFD driver to the driver for the actual sub-functions.
> Examples are:

There absolutely is.  You can select IPMI_LS2K without selecting
IPMI_SI.  It would compile without it, and it just wouldn't do anything,
but it would be confusing to be able to select something and it not do
anything.

> 
> - MFD_MENF21BMC and SENSORS_MENF21BMC_HWMON, LEDS_MENF21BMC, 
>   MENF21BMC_WATCHDOG: The later three declared depends on the former MFD
>   driver
> - MFD_INTEL_M10_BMC_CORE and FPGA_M10_BMC_SEC_UPDATE, 
>   SENSORS_INTEL_M10_BMC_HWMON: The later two declared depends on the
>   former MFD driver.
> - MFD_STMFX and PINCTRL_STMFX: The later declared select the former
>   MFD driver.
> 
> To summarize, as shown in the current mfd drivers, there is no need to
> declare dependency to the drivers providing the sub-functions in the
> mfd drivers, and the drivers of the sub functions should "depends on" or
> "select" the mfd driver. So if IPMI_LS2K could be tristate, then it would
> be reasonable to select MFD_LS2K_BMC_CORE or depends on MFD_LS2K_BMC_CORE.
> 
> 
> Regarding the fact that IPMI_LS2K is bool and it is not an independent
> module but a part of ipmi_si, it comes into my mind that we may choose
> to adjust the dependency from ipmi_si. That is to remove "select
> MFD_LS2K_BMC_CORE" from IPMI_LS2K, and add "select MFD_LS2K_BMC_CORE
> if IPMI_LS2K" to IPMI_SI.
> 
> I wonder if such a change would be appropriate.

That would be a good solution if it works.  Thanks for working with
me on this.

-corey

> 
> Cheers,
> 
> Miao Wang
>