Message-ID: <CAMpQs4L66sXLdj6+ebMXcwq4i+XVi+AmwBbQSx0-EuULNJ5XeQ@mail.gmail.com>
Date: Sat, 14 Jun 2025 10:50:37 +0800
From: Binbin Zhou <zhoubb.aaron@...il.com>
To: corey@...yard.net
Cc: Binbin Zhou <zhoubinbin@...ngson.cn>, Huacai Chen <chenhuacai@...ngson.cn>, 
	Lee Jones <lee@...nel.org>, Corey Minyard <minyard@....org>, Huacai Chen <chenhuacai@...nel.org>, 
	Xuerui Wang <kernel@...0n.name>, loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	openipmi-developer@...ts.sourceforge.net, jeffbai@...c.io, 
	kexybiscuit@...c.io, wangyao@...ote.com
Subject: Re: [PATCH v4 0/3] LoongArch: Add Loongson-2K BMC support

Hi Corey:

On Sat, Jun 14, 2025 at 8:41 AM Corey Minyard <corey@...yard.net> wrote:
>
> On Fri, Jun 13, 2025 at 02:43:38PM +0800, Binbin Zhou wrote:
> > Hi all:
> >
> > This patch set introduces the Loongson-2K BMC.
> >
> > It is a PCIe device present on servers similar to the Loongson-3 CPUs.
> > And it is a multifunctional device (MFD), such as display as a sub-function
> > of it.
>
> I've asked this before, but I haven't gotten a answer, I don't think.
>
> Is this really a multi-function device?  Is there (or will there be)
> another driver that uses the MFD code?

I am very sorry, I may have overlooked your previous question.

And I also may not have a thorough understanding of multifunction devices.

The Loongson-2K BMC device provides two functions: display and IPMI.
For display, we pass the simplefb_platform_data parameter and register
the simpledrm device, as shown in patch-1.

Therefore, I think this should be considered a multifunction device.

>
> If nothing else is going to use this, then it's really not a
> multi-function device and all the code can go into the IPMI directory.
> That simplifies maintenance.
>
> If it is a multi-function device, then I want two separate Kconfig
> items, one for the MFD and one for the IPMI portion.  That way it's
> ready and you don't have to bother about the IPMI portion when
> adding the other device.
>
> All else looks good, I think.
>
> -corey
>
> >
> > For IPMI, according to the existing design, we use software simulation to
> > implement the KCS interface registers: Stauts/Command/Data_Out/Data_In.
> >
> > Also since both host side and BMC side read and write kcs status, we use
> > fifo pointer to ensure data consistency.
> >
> > For the display, based on simpledrm, the resolution is read from a fixed
> > position in the BMC since the hardware does not support auto-detection
> > of the resolution. Of course, we will try to support multiple
> > resolutions later, through a vbios-like approach.
> >
> > Especially, for the BMC reset function, since the display will be
> > disconnected when BMC reset, we made a special treatment of re-push.
> >
> > Based on this, I will present it in four patches:
> > patch-1: BMC device PCI resource allocation.
> > patch-2: BMC reset function support
> > patch-3: IPMI implementation
> >
> > Thanks.
> >
> > -------
> > V4:
> > - Add Reviewed-by tag;
> > - Change the order of the patches.
> > Patch (1/3):
> >   - Fix build warning by lkp: Kconfig tristate -> bool
> >     - https://lore.kernel.org/all/202505312022.QmFmGE1F-lkp@intel.com/
> >  - Update commit message;
> >  - Move MFD_LS2K_BMC after MFD_INTEL_M10_BMC_PMCI in Kconfig and
> >    Makefile.
> > Patch (2/3):
> >   - Remove unnecessary newlines;
> >   - Rename ls2k_bmc_check_pcie_connected() to
> >     ls2k_bmc_pcie_is_connected();
> >   - Update comment message.
> > Patch (3/3):
> >   - Remove unnecessary newlines.
> >
> > Link to V3:
> > https://lore.kernel.org/all/cover.1748505446.git.zhoubinbin@loongson.cn/
> >
> > V3:
> > Patch (1/3):
> >  - Drop "MFD" in title and comment;
> >  - Fromatting code;
> >  - Add clearer comments.
> > Patch (2/3):
> >  - Rebase linux-ipmi/next tree;
> >  - Use readx()/writex() to read and write IPMI data instead of structure
> >    pointer references;
> >  - CONFIG_LOONGARCH -> MFD_LS2K_BMC;
> >  - Drop unused output.
> > Patch (3/3):
> >  - Inline the ls2k_bmc_gpio_reset_handler() function to ls2k_bmc_pdata_initial();
> >  - Add clearer comments.
> >  - Use proper multi-line commentary as per the Coding Style documentation;
> >  - Define all magic numbers.
> >
> > Link to V2:
> > https://lore.kernel.org/all/cover.1747276047.git.zhoubinbin@loongson.cn/
> >
> > V2:
> > - Drop ls2kdrm, use simpledrm instead.
> > Patch (1/3):
> >  - Use DEFINE_RES_MEM_NAMED/MFD_CELL_RES simplified code;
> >  - Add resolution fetching due to replacing the original display
> >    solution with simpledrm;
> >  - Add aperture_remove_conflicting_devices() to avoid efifb
> >    conflict with simpledrm.
> > Patch (3/3):
> >  - This part of the function, moved from the original ls2kdrm to mfd;
> >  - Use set_console to implement the Re-push display function.
> >
> > Link to V1:
> > https://lore.kernel.org/all/cover.1735550269.git.zhoubinbin@loongson.cn/
> >
> > Binbin Zhou (3):
> >   mfd: ls2kbmc: Introduce Loongson-2K BMC core driver
> >   mfd: ls2kbmc: Add Loongson-2K BMC reset function support
> >   ipmi: Add Loongson-2K BMC support
> >
> >  drivers/char/ipmi/Makefile       |   1 +
> >  drivers/char/ipmi/ipmi_si.h      |   7 +
> >  drivers/char/ipmi/ipmi_si_intf.c |   3 +
> >  drivers/char/ipmi/ipmi_si_ls2k.c | 189 ++++++++++++
> >  drivers/mfd/Kconfig              |  12 +
> >  drivers/mfd/Makefile             |   2 +
> >  drivers/mfd/ls2kbmc-mfd.c        | 485 +++++++++++++++++++++++++++++++
> >  7 files changed, 699 insertions(+)
> >  create mode 100644 drivers/char/ipmi/ipmi_si_ls2k.c
> >  create mode 100644 drivers/mfd/ls2kbmc-mfd.c
> >
> >
> > base-commit: cd2e103d57e5615f9bb027d772f93b9efd567224
> > --
> > 2.47.1
> >


-- 
Thanks.
Binbin

Powered by blists - more mailing lists