Re: [PATCH RFC v3 0/7] mfd: ls2kbmc: multiple fixes for this driver
From: Miao Wang
Date: Mon Aug 03 2026 - 12:42:56 EST
Hi,
> 2026年8月3日 21:45,Huacai Chen <chenhuacai@xxxxxxxxxx> 写道:
>
> On Fri, Jul 31, 2026 at 4:24 PM Miao Wang <shankerwangmiao@xxxxxxxxx> wrote:
>>
>> Hi,
>>
>>> 2026年7月31日 16:07,Huacai Chen <chenhuacai@xxxxxxxxxx> 写道:
>>>
>>> On Fri, Jul 24, 2026 at 5:28 PM Miao Wang <shankerwangmiao@xxxxxxxxx> wrote:
>>>>
>>>> Hi,
>>>>
>>>>> 2026年7月24日 16:55,Huacai Chen <chenhuacai@xxxxxxxxxx> 写道:
>>>>>
>>>>> Hi, Miao,
>>>>>
>>>>> On Fri, Jul 10, 2026 at 1:24 AM Miao Wang via B4 Relay
>>>>> <devnull+shankerwangmiao.gmail.com@xxxxxxxxxx> 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 default
>>>>>> efi-framebuffer is being used. However, there are some drawbacks in 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 PCI-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 necessary
>>>>> 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 better
>>>> 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 ipmi
>>>> device. In the aspect of graphical display, without this module, the
>>>> user can still see the output during booting on their monitors, since
>>>> 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 influence
>>>> the boot of the system, since it is a common practice to compile ipmi
>>>> device drivers as modules on other architectures. As a result, neither
>>>> 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 modular?
>
> 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.
Cheers,
Miao Wang