Re: [PATCH RFC v3 0/7] mfd: ls2kbmc: multiple fixes for this driver

On Tue, Aug 4, 2026 at 12:09=E2=80=AFAM Miao Wang <[email protected]=
m> wrote:
>
> Hi,
>
> > 2026=E5=B9=B48=E6=9C=883=E6=97=A5 21:45=EF=BC=8CHuacai Chen <chenhuacai=
@kernel.org> =E5=86=99=E9=81=93=EF=BC=9A
> >
> > On Fri, Jul 31, 2026 at 4:24=E2=80=AFPM Miao Wang <shankerwangmiao@gmai=
l.com> wrote:
> >>
> >> Hi,
> >>
> >>> 2026=E5=B9=B47=E6=9C=8831=E6=97=A5 16:07=EF=BC=8CHuacai Chen <chenhua=
[email protected]> =E5=86=99=E9=81=93=EF=BC=9A
> >>>
> >>> On Fri, Jul 24, 2026 at 5:28=E2=80=AFPM Miao Wang <shankerwangmiao@gm=
ail.com> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>>> 2026=E5=B9=B47=E6=9C=8824=E6=97=A5 16:55=EF=BC=8CHuacai Chen <chenh=
[email protected]> =E5=86=99=E9=81=93=EF=BC=9A
> >>>>>
> >>>>> Hi, Miao,
> >>>>>
> >>>>> On Fri, Jul 10, 2026 at 1:24=E2=80=AFAM Miao Wang via B4 Relay
> >>>>> <[email protected]> wrote:
> >>>>>>
> >>>>>> Previously, the driver has been introduced to support the Loongson=
 2K
> >>>>>> BMC running on the Loongson Servers, which is essential to prevent
> >>>>>> the system from hanging when the BMC is being reset and the defaul=
t
> >>>>>> efi-framebuffer is being used. However, there are some drawbacks i=
n the
> >>>>>> driver.
> >>>>>>
> >>>>>> Firstly, the driver tries to read and write to the connected PCI-E=
 host
> >>>>>> controller registers, assuming that the BMC is connected to LS7A P=
CI-E
> >>>>>> host controller. This assumption should be true for real products,=
 but
> >>>>>> to prevent from accidentally reading and writing to the wrong PCI-=
E host
> >>>>>> controller, this driver should be modified to check this before
> >>>>>> accessing the registers.
> >>>>>>
> >>>>>> Secondly, the driver uses non-exported functions to tell the vt
> >>>>>> subsystem to redraw the screen, preventing the driver from being
> >>>>>> compiling as a module. This can be fixed by using the exported
> >>>>>> functions instead.
> >>>>> You can replace the redraw function, but I don't think it is necess=
ary
> >>>>> to make the bmc driver modular.
> >>>>>
> >>>>> BMC core, IPMI and simpledrm display are usually (if not always)
> >>>>> supposed to work as early as possible.
> >>>>
> >>>> I believe that it should be the user's decision to choose whether to
> >>>> compile a module into the kernel or as a module and it would be bett=
er
> >>>> if we can provide the possibilities for the user to choose from.
> >>>> Additionally, I don't think these modules are supposed to work that
> >>>> early. The mfd module provide two functions, the display and the ipm=
i
> >>>> device. In the aspect of graphical display, without this module, the
> >>>> user can still see the output during booting on their monitors, sinc=
e
> >>>> efifb is working, providing a basic display function. In the aspect =
of
> >>>> the ipmi device, I don't think the lack of such device will influenc=
e
> >>>> the boot of the system, since it is a common practice to compile ipm=
i
> >>>> device drivers as modules on other architectures. As a result, neith=
er
> >>>> of the two functions are required to be loaded that early and it is
> >>>> reasonable to at lease leave the choice to compile it as a module
> >>>> to the user.
> >>> Flexibility is not always useful, if a config doesn't provide good
> >>> effect, then it just increases complexity and makes maintenance more
> >>> difficult.
> >>
> >> I should emphasize that to allow this driver to be a module, there is
> >> no such increase on maintenance. Moreover, not all loongarch machines
> >> are requiring this driver. Especially only a part of the server models
> >> are quipped with this BMC. Comparing with other architectures, the
> >> driver for BMC are normally compiled as a module, such as mgag200 for
> >> iLO from HPE and iDRAC from DELL, hibmc_drm for Kunpeng server from
> >> Huawei. None of these BMC drivers requiring to be compiled into the
> >> kernel. I cannot see there is any reason keeping the driver from being
> >> allowed to be compiled as a module. I also do not think it will bring
> >> any significant maintenance burden. Implementing correct cleanup code
> >> should be necessary instead of burden.
> > Can we split into two series, one fix bugs and the others make bmc modu=
lar?
> >
> > Otherwise I don't think we can reach a consensus in the near future.
>
> I accept different opinions on design trade-offs. However, I don't think
> you have provided enough excuses to remain this driver as built-in,
> since I believe normally in kernel, most non-core drivers are all able
> to be compiled as a module. I also provided some examples from devices
> with similar functions. As a result, I'll not split this series before
> there is indeed a strong reason against allowing this driver to be
> compiled as a module or we may have a great benefit if we force this
> module to be compiled built-in.
At least you should remove the first patch from this series. Binbin
Zhou has been fixing it:
https://lore.kernel.org/loongarch/20260616115530.4012675-1-zhoubinbin@loong=
son.cn/
https://lore.kernel.org/loongarch/20260624085550.1508771-1-zhoubinbin@loong=
son.cn/
https://lore.kernel.org/loongarch/20260706021909.2346535-1-zhoubinbin@loong=
son.cn/

Huacai

>
> Cheers,
>
> Miao Wang