[PATCH v2 0/3] LoongArch: Add Loongson-2K0500 BMC support

[PATCH v2 0/3] LoongArch: Add Loongson-2K0500 BMC support

[Binbin Zhou]

Hi all:

This patch set introduces the Loongson-2K0500 BMC.

It is a PCIe device present on servers similar to the Loongson-3C6000.
And it is a multifunctional device (MFD), such as display as a sub-function
of it.

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: IPMI implementation
patch-3: display, based on simpledrm
patch-4: BMC reboot support

Thanks.

-------
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 MFD Core driver
  ipmi: Add Loongson-2K BMC support
  mfd: ls2kbmc: Add Loongson-2K BMC reset function 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 | 250 +++++++++++++++++++
 drivers/mfd/Kconfig              |  13 +
 drivers/mfd/Makefile             |   2 +
 drivers/mfd/ls2kbmc-mfd.c        | 398 +++++++++++++++++++++++++++++++
 7 files changed, 674 insertions(+)
 create mode 100644 drivers/char/ipmi/ipmi_si_ls2k.c
 create mode 100644 drivers/mfd/ls2kbmc-mfd.c


base-commit: 9f2b0c15b752bb940e2eb6737bee30fff96d96b6
-- 
2.47.1

[PATCH v2 1/3] mfd: ls2kbmc: Introduce Loongson-2K BMC MFD Core driver

[Binbin Zhou]

The Loongson-2K Board Management Controller provides an PCIe
interface to the host to access the feature implemented in the BMC.

The BMC is assembled on a server similar to the server machine with
Loongson-3C6000 CPUs. It supports multiple sub-devices like DRM.

Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/mfd/Kconfig       |  13 ++++
 drivers/mfd/Makefile      |   2 +
 drivers/mfd/ls2kbmc-mfd.c | 156 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 171 insertions(+)
 create mode 100644 drivers/mfd/ls2kbmc-mfd.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 22b936310039..04e40085441d 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2422,5 +2422,18 @@ config MFD_UPBOARD_FPGA
 	  To compile this driver as a module, choose M here: the module will be
 	  called upboard-fpga.
 
+config MFD_LS2K_BMC
+	tristate "Loongson-2K Board Management Controller Support"
+	depends on LOONGARCH
+	default y if LOONGARCH
+	select MFD_CORE
+	help
+	  Say yes here to add support for the Loongson-2K BMC
+	  which is a Board Management Controller connected to the PCIe bus.
+	  The device supports multiple sub-devices like DRM.
+	  This driver provides common support for accessing the devices;
+	  additional drivers must be enabled in order to use the
+	  functionality of the BMC device.
+
 endmenu
 endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 948cbdf42a18..18960ea13b64 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -290,3 +290,5 @@ obj-$(CONFIG_MFD_RSMU_I2C)	+= rsmu_i2c.o rsmu_core.o
 obj-$(CONFIG_MFD_RSMU_SPI)	+= rsmu_spi.o rsmu_core.o
 
 obj-$(CONFIG_MFD_UPBOARD_FPGA)	+= upboard-fpga.o
+
+obj-$(CONFIG_MFD_LS2K_BMC)	+= ls2kbmc-mfd.o
diff --git a/drivers/mfd/ls2kbmc-mfd.c b/drivers/mfd/ls2kbmc-mfd.c
new file mode 100644
index 000000000000..b309f6132c24
--- /dev/null
+++ b/drivers/mfd/ls2kbmc-mfd.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Loongson-2K Board Management Controller (BMC) MFD Core Driver.
+ *
+ * Copyright (C) 2024 Loongson Technology Corporation Limited.
+ *
+ * Originally written by Chong Qiao <qiaochong@loongson.cn>
+ * Rewritten for mainline by Binbin Zhou <zhoubinbin@loongson.cn>
+ */
+
+#include <linux/aperture.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/pci_ids.h>
+#include <linux/platform_data/simplefb.h>
+#include <linux/platform_device.h>
+
+#define LS2K_DISPLAY_RES_START (SZ_16M + SZ_2M)
+#define LS2K_IPMI_RES_SIZE	0x1c
+#define LS2K_IPMI0_RES_START	(SZ_16M + 0xf00000)
+#define LS2K_IPMI1_RES_START	(LS2K_IPMI0_RES_START + LS2K_IPMI_RES_SIZE)
+#define LS2K_IPMI2_RES_START	(LS2K_IPMI1_RES_START + LS2K_IPMI_RES_SIZE)
+#define LS2K_IPMI3_RES_START	(LS2K_IPMI2_RES_START + LS2K_IPMI_RES_SIZE)
+#define LS2K_IPMI4_RES_START	(LS2K_IPMI3_RES_START + LS2K_IPMI_RES_SIZE)
+
+static struct resource ls2k_display_resources[] = {
+	DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
+};
+
+static struct resource ls2k_ipmi0_resources[] = {
+	DEFINE_RES_MEM_NAMED(LS2K_IPMI0_RES_START, LS2K_IPMI_RES_SIZE, "ipmi0-res"),
+};
+
+static struct resource ls2k_ipmi1_resources[] = {
+	DEFINE_RES_MEM_NAMED(LS2K_IPMI1_RES_START, LS2K_IPMI_RES_SIZE, "ipmi1-res"),
+};
+
+static struct resource ls2k_ipmi2_resources[] = {
+	DEFINE_RES_MEM_NAMED(LS2K_IPMI2_RES_START, LS2K_IPMI_RES_SIZE, "ipmi2-res"),
+};
+
+static struct resource ls2k_ipmi3_resources[] = {
+	DEFINE_RES_MEM_NAMED(LS2K_IPMI3_RES_START, LS2K_IPMI_RES_SIZE, "ipmi3-res"),
+};
+
+static struct resource ls2k_ipmi4_resources[] = {
+	DEFINE_RES_MEM_NAMED(LS2K_IPMI4_RES_START, LS2K_IPMI_RES_SIZE, "ipmi4-res"),
+};
+
+static struct mfd_cell ls2k_bmc_cells[] = {
+	MFD_CELL_RES("simple-framebuffer", ls2k_display_resources),
+	MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi0_resources),
+	MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi1_resources),
+	MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi2_resources),
+	MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi3_resources),
+	MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
+};
+
+/*
+ * Currently the Loongson-2K0500 BMC hardware does not have an i2c interface to
+ * adapt to the resolution.
+ * We set the resolution by presetting "video=1280x1024-16@2M" to the bmc memory.
+ */
+static int ls2k_bmc_get_video_mode(struct pci_dev *pdev, struct simplefb_platform_data *pd)
+{
+	char *mode;
+	int depth, ret;
+
+	/* The pci mem bar last 16M is used to store the string. */
+	mode = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0) + SZ_16M, SZ_16M);
+	if (!mode)
+		return -ENOMEM;
+
+	/* env at last 16M's beginning, first env is "video=" */
+	if (!strncmp(mode, "video=", 6))
+		mode = mode + 6;
+
+	ret = kstrtoint(strsep(&mode, "x"), 10, &pd->width);
+	if (ret)
+		return ret;
+
+	ret = kstrtoint(strsep(&mode, "-"), 10, &pd->height);
+	if (ret)
+		return ret;
+
+	ret = kstrtoint(strsep(&mode, "@"), 10, &depth);
+	if (ret)
+		return ret;
+
+	pd->stride = pd->width * depth / 8;
+	pd->format = depth == 32 ? "a8r8g8b8" : "r5g6b5";
+
+	return 0;
+}
+
+static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
+{
+	int ret = 0;
+	resource_size_t base;
+	struct simplefb_platform_data pd;
+
+	ret = pci_enable_device(dev);
+	if (ret)
+		return ret;
+
+	ret = ls2k_bmc_get_video_mode(dev, &pd);
+	if (ret)
+		goto disable_pci;
+
+	ls2k_bmc_cells[0].platform_data = &pd;
+	ls2k_bmc_cells[0].pdata_size = sizeof(pd);
+	base = dev->resource[0].start + LS2K_DISPLAY_RES_START;
+
+	/* Remove conflicting efifb device */
+	ret = aperture_remove_conflicting_devices(base, SZ_4M, "simple-framebuffer");
+	if (ret) {
+		dev_err(&dev->dev, "Remove firmware framebuffers failed: %d\n", ret);
+		goto disable_pci;
+	}
+
+	return devm_mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
+				    ls2k_bmc_cells, ARRAY_SIZE(ls2k_bmc_cells),
+				    &dev->resource[0], 0, NULL);
+
+disable_pci:
+	pci_disable_device(dev);
+	return ret;
+}
+
+static void ls2k_bmc_remove(struct pci_dev *dev)
+{
+	pci_disable_device(dev);
+}
+
+static struct pci_device_id ls2k_bmc_devices[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x1a05) },
+	{ }
+};
+MODULE_DEVICE_TABLE(pci, ls2k_bmc_devices);
+
+static struct pci_driver ls2k_bmc_driver = {
+	.name = "ls2k-bmc",
+	.id_table = ls2k_bmc_devices,
+	.probe = ls2k_bmc_probe,
+	.remove = ls2k_bmc_remove,
+};
+
+module_pci_driver(ls2k_bmc_driver);
+
+MODULE_DESCRIPTION("Loongson-2K BMC driver");
+MODULE_AUTHOR("Loongson Technology Corporation Limited");
+MODULE_LICENSE("GPL");
-- 
2.47.1

[PATCH v2 2/3] ipmi: Add Loongson-2K BMC support

[Binbin Zhou]

This patch adds Loongson-2K BMC IPMI support.

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, I use
fifo pointer to ensure data consistency.

Therefore I made the whole IPMI driver independent.

Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 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 | 250 +++++++++++++++++++++++++++++++
 4 files changed, 261 insertions(+)
 create mode 100644 drivers/char/ipmi/ipmi_si_ls2k.c

diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
index e0944547c9d0..5eb3494f5f39 100644
--- a/drivers/char/ipmi/Makefile
+++ b/drivers/char/ipmi/Makefile
@@ -8,6 +8,7 @@ ipmi_si-y := ipmi_si_intf.o ipmi_kcs_sm.o ipmi_smic_sm.o ipmi_bt_sm.o \
 	ipmi_si_mem_io.o
 ipmi_si-$(CONFIG_HAS_IOPORT) += ipmi_si_port_io.o
 ipmi_si-$(CONFIG_PCI) += ipmi_si_pci.o
+ipmi_si-$(CONFIG_LOONGARCH) += ipmi_si_ls2k.o
 ipmi_si-$(CONFIG_PARISC) += ipmi_si_parisc.o
 
 obj-$(CONFIG_IPMI_HANDLER) += ipmi_msghandler.o
diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h
index a7ead2a4c753..71f1d4e1272c 100644
--- a/drivers/char/ipmi/ipmi_si.h
+++ b/drivers/char/ipmi/ipmi_si.h
@@ -93,6 +93,13 @@ void ipmi_si_pci_shutdown(void);
 static inline void ipmi_si_pci_init(void) { }
 static inline void ipmi_si_pci_shutdown(void) { }
 #endif
+#ifdef CONFIG_LOONGARCH
+void ipmi_si_ls2k_init(void);
+void ipmi_si_ls2k_shutdown(void);
+#else
+static inline void ipmi_si_ls2k_init(void) { }
+static inline void ipmi_si_ls2k_shutdown(void) { }
+#endif
 #ifdef CONFIG_PARISC
 void ipmi_si_parisc_init(void);
 void ipmi_si_parisc_shutdown(void);
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 12b0b77eb1cc..323da77698ea 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -2107,6 +2107,7 @@ static int __init init_ipmi_si(void)
 
 	ipmi_si_pci_init();
 
+	ipmi_si_ls2k_init();
 	ipmi_si_parisc_init();
 
 	/* We prefer devices with interrupts, but in the case of a machine
@@ -2288,6 +2289,8 @@ static void cleanup_ipmi_si(void)
 
 	ipmi_si_pci_shutdown();
 
+	ipmi_si_ls2k_shutdown();
+
 	ipmi_si_parisc_shutdown();
 
 	ipmi_si_platform_shutdown();
diff --git a/drivers/char/ipmi/ipmi_si_ls2k.c b/drivers/char/ipmi/ipmi_si_ls2k.c
new file mode 100644
index 000000000000..cb31bb989fca
--- /dev/null
+++ b/drivers/char/ipmi/ipmi_si_ls2k.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Loongson-2K BMC IPMI
+ *
+ * Copyright (C) 2024 Loongson Technology Corporation Limited.
+ *
+ * Originally written by Chong Qiao <qiaochong@loongson.cn>
+ * Rewritten for mainline by Binbin Zhou <zhoubinbin@loongson.cn>
+ */
+
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/types.h>
+
+#include "ipmi_si.h"
+
+#define LS2K_KCS_STS_OBF	BIT(0)
+#define LS2K_KCS_STS_IBF	BIT(1)
+#define LS2K_KCS_STS_SMS_ATN	BIT(2)
+#define LS2K_KCS_STS_CMD	BIT(3)
+
+#define LS2K_KCS_DATA_MASK	(LS2K_KCS_STS_OBF | LS2K_KCS_STS_IBF | LS2K_KCS_STS_CMD)
+
+/* Read and write fifo pointers for data consistency. */
+struct ls2k_fifo_flag {
+	u8 ibfh;
+	u8 ibft;
+	u8 obfh;
+	u8 obft;
+};
+
+struct ls2k_kcs_reg {
+	u8 status;
+	u8 data_out;
+	s16 data_in;
+	s16 cmd;
+};
+
+struct ls2k_kcs_data {
+	struct ls2k_fifo_flag fifo;
+	struct ls2k_kcs_reg reg;
+	u8 cmd_data;
+	u8 version;
+	u32 write_req;
+	u32 write_ack;
+	u32 reserved[2];
+};
+
+static void ls2k_set_obf(struct ls2k_kcs_data *ik, u8 sts)
+{
+	ik->reg.status = (ik->reg.status & ~LS2K_KCS_STS_OBF) | (sts & BIT(0));
+}
+
+static void ls2k_set_ibf(struct ls2k_kcs_data *ik, u8 sts)
+{
+	ik->reg.status = (ik->reg.status & ~LS2K_KCS_STS_IBF) | ((sts & BIT(0)) << 1);
+}
+
+static u8 ls2k_get_ibf(struct ls2k_kcs_data *ik)
+{
+	return (ik->reg.status >> 1) & BIT(0);
+}
+
+static unsigned char intf_sim_inb_v0(struct ls2k_kcs_data *ik,
+				     unsigned int offset)
+{
+	u32 inb = 0;
+
+	switch (offset & BIT(0)) {
+	case 0:
+		inb = ik->reg.data_out;
+		ls2k_set_obf(ik, 0);
+		break;
+	case 1:
+		inb = ik->reg.status;
+		break;
+	}
+
+	return inb;
+}
+
+static unsigned char intf_sim_inb_v1(struct ls2k_kcs_data *ik,
+				     unsigned int offset)
+{
+	u32 inb = 0;
+	int cmd;
+	bool obf, ibf;
+
+	obf = ik->fifo.obfh != ik->fifo.obft;
+	ibf = ik->fifo.ibfh != ik->fifo.ibft;
+	cmd = ik->cmd_data;
+
+	switch (offset & BIT(0)) {
+	case 0:
+		inb = ik->reg.data_out;
+		ik->fifo.obft = ik->fifo.obfh;
+		break;
+	case 1:
+		inb = ik->reg.status & ~LS2K_KCS_DATA_MASK;
+		inb |= obf | (ibf << 1) | (cmd << 3);
+		break;
+	}
+
+	return inb;
+}
+
+static unsigned char ls2k_mem_inb(const struct si_sm_io *io,
+				  unsigned int offset)
+{
+	struct ls2k_kcs_data *ik = io->addr;
+	int inb = 0;
+
+	if (ik->version == 0)
+		inb = intf_sim_inb_v0(ik, offset);
+	else if (ik->version == 1)
+		inb = intf_sim_inb_v1(ik, offset);
+
+	return inb;
+}
+
+static void intf_sim_outb_v0(struct ls2k_kcs_data *ik, unsigned int offset,
+			     unsigned char val)
+{
+	if (ls2k_get_ibf(ik))
+		return;
+
+	switch (offset & BIT(0)) {
+	case 0:
+		ik->reg.data_in = val;
+		ik->reg.status &= ~LS2K_KCS_STS_CMD;
+		break;
+
+	case 1:
+		ik->reg.cmd = val;
+		ik->reg.status |= LS2K_KCS_STS_CMD;
+		break;
+	}
+
+	ls2k_set_ibf(ik, 1);
+	ik->write_req++;
+}
+
+static void intf_sim_outb_v1(struct ls2k_kcs_data *ik, unsigned int offset,
+			     unsigned char val)
+{
+	if (ik->fifo.ibfh != ik->fifo.ibft)
+		return;
+
+	switch (offset & BIT(0)) {
+	case 0:
+		ik->reg.data_in = val;
+		ik->cmd_data = 0;
+		break;
+
+	case 1:
+		ik->reg.cmd = val;
+		ik->cmd_data = 1;
+		break;
+	}
+
+	ik->fifo.ibfh = !ik->fifo.ibft;
+	ik->write_req++;
+}
+
+static void ls2k_mem_outb(const struct si_sm_io *io, unsigned int offset,
+			  unsigned char val)
+{
+	struct ls2k_kcs_data *ik = io->addr;
+
+	if (ik->version == 0)
+		intf_sim_outb_v0(ik, offset, val);
+	else if (ik->version == 1)
+		intf_sim_outb_v1(ik, offset, val);
+}
+
+static void ls2k_mem_cleanup(struct si_sm_io *io)
+{
+	if (io->addr)
+		iounmap(io->addr);
+}
+
+static int ipmi_ls2k_sim_setup(struct si_sm_io *io)
+{
+	io->addr = ioremap(io->addr_data, io->regspacing);
+	if (!io->addr)
+		return -EIO;
+
+	io->inputb = ls2k_mem_inb;
+	io->outputb = ls2k_mem_outb;
+	io->io_cleanup = ls2k_mem_cleanup;
+
+	return 0;
+}
+
+static int ipmi_ls2k_probe(struct platform_device *pdev)
+{
+	struct si_sm_io io;
+
+	dev_info(&pdev->dev, "probing via ls2k platform");
+	memset(&io, 0, sizeof(io));
+
+	io.addr_source	= SI_PLATFORM;
+	io.si_type	= SI_KCS;
+	io.addr_space	= IPMI_MEM_ADDR_SPACE;
+	io.io_setup	= ipmi_ls2k_sim_setup;
+	io.addr_data	= pdev->resource[0].start;
+	io.regspacing	= pdev->resource[0].end - pdev->resource[0].start + 1;
+	io.regsize	= DEFAULT_REGSIZE;
+	io.regshift	= 0;
+	io.dev		= &pdev->dev;
+	io.irq		= 0;
+	if (io.irq)
+		io.irq_setup = ipmi_std_irq_setup;
+
+	dev_info(&pdev->dev, "%pR regsize %d spacing %d irq %d\n",
+		 &pdev->resource[0], io.regsize, io.regspacing, io.irq);
+
+	return ipmi_si_add_smi(&io);
+}
+
+static void ipmi_ls2k_remove(struct platform_device *pdev)
+{
+	ipmi_si_remove_by_dev(&pdev->dev);
+}
+
+struct platform_driver ipmi_ls2k_platform_driver = {
+	.driver = {
+		.name = "ls2k-ipmi-si",
+	},
+	.probe	= ipmi_ls2k_probe,
+	.remove	= ipmi_ls2k_remove,
+};
+
+static bool platform_registered;
+void ipmi_si_ls2k_init(void)
+{
+	int rv;
+
+	rv = platform_driver_register(&ipmi_ls2k_platform_driver);
+	if (rv)
+		pr_err("Unable to register driver: %d\n", rv);
+	else
+		platform_registered = true;
+}
+
+void ipmi_si_ls2k_shutdown(void)
+{
+	if (platform_registered)
+		platform_driver_unregister(&ipmi_ls2k_platform_driver);
+}
-- 
2.47.1

[PATCH v2 3/3] mfd: ls2kbmc: Add Loongson-2K BMC reset function support

[Binbin Zhou]

Since the display is a sub-function of the Loongson-2K BMC, when the
BMC reset, the entire BMC PCIe is disconnected, including the display
which is interrupted.

Not only do you need to save/restore the relevant PCIe registers
throughout the reset process, but you also need to re-push the display
to the monitor at the end.

Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/mfd/ls2kbmc-mfd.c | 242 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 242 insertions(+)

diff --git a/drivers/mfd/ls2kbmc-mfd.c b/drivers/mfd/ls2kbmc-mfd.c
index b309f6132c24..4d35a13b3da5 100644
--- a/drivers/mfd/ls2kbmc-mfd.c
+++ b/drivers/mfd/ls2kbmc-mfd.c
@@ -9,8 +9,11 @@
  */
 
 #include <linux/aperture.h>
+#include <linux/delay.h>
 #include <linux/errno.h>
 #include <linux/init.h>
+#include <linux/iopoll.h>
+#include <linux/kbd_kern.h>
 #include <linux/kernel.h>
 #include <linux/mfd/core.h>
 #include <linux/module.h>
@@ -18,6 +21,8 @@
 #include <linux/pci_ids.h>
 #include <linux/platform_data/simplefb.h>
 #include <linux/platform_device.h>
+#include <linux/stop_machine.h>
+#include <linux/vt_kern.h>
 
 #define LS2K_DISPLAY_RES_START (SZ_16M + SZ_2M)
 #define LS2K_IPMI_RES_SIZE	0x1c
@@ -27,6 +32,9 @@
 #define LS2K_IPMI3_RES_START	(LS2K_IPMI2_RES_START + LS2K_IPMI_RES_SIZE)
 #define LS2K_IPMI4_RES_START	(LS2K_IPMI3_RES_START + LS2K_IPMI_RES_SIZE)
 
+#define LS2K_BMC_RESET_DELAY	(60 * HZ)
+#define LS2K_BMC_RESET_WAIT	(10 * HZ)
+
 static struct resource ls2k_display_resources[] = {
 	DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
 };
@@ -60,6 +68,227 @@ static struct mfd_cell ls2k_bmc_cells[] = {
 	MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
 };
 
+static const u32 index[] = { 0x4, 0x10, 0x14, 0x18, 0x1c, 0x20, 0x24,
+			     0x30, 0x3c, 0x54, 0x58, 0x78, 0x7c, 0x80 };
+static const u32 cindex[] = { 0x4, 0x10, 0x3c };
+
+struct ls2k_bmc_pci_data {
+	u32 d80c;
+	u32 d71c;
+	u32 data[14];
+	u32 cdata[3];
+};
+
+struct ls2k_bmc_pdata {
+	struct device *dev;
+	struct work_struct reset_work;
+	unsigned long reset_time;
+	struct ls2k_bmc_pci_data pci_data;
+};
+
+static bool ls2k_bmc_bar0_addr_is_set(struct pci_dev *ppdev)
+{
+	u32 addr;
+
+	pci_read_config_dword(ppdev, PCI_BASE_ADDRESS_0, &addr);
+	addr &= PCI_BASE_ADDRESS_MEM_MASK;
+
+	return addr ? true : false;
+}
+
+static bool ls2k_bmc_check_pcie_connected(struct pci_dev *parent,
+					  struct ls2k_bmc_pdata *priv)
+{
+	void __iomem *mmio;
+	int sts, ret;
+
+	mmio = pci_iomap(parent, 0, 0x100);
+	if (!mmio)
+		return false;
+
+	writel(readl(mmio) | 0x8, mmio);
+
+	ret = readl_poll_timeout_atomic(mmio + 0xc, sts, (sts & 0x11) == 0x11,
+					1000, 1000000);
+	if (ret) {
+		pci_iounmap(parent, mmio);
+		dev_err(priv->dev, "PCIE train failed status=0x%x\n", sts);
+		return false;
+	}
+
+	pci_iounmap(parent, mmio);
+	return true;
+}
+
+static int ls2k_bmc_recover_pci_data(void *data)
+{
+	struct ls2k_bmc_pdata *priv = data;
+	struct pci_dev *pdev = to_pci_dev(priv->dev);
+	struct pci_dev *parent = pdev->bus->self;
+	bool ready, dirty;
+	u32 i;
+
+	pci_write_config_dword(parent, PCI_BASE_ADDRESS_2, 0);
+	pci_write_config_dword(parent, PCI_BASE_ADDRESS_3, 0);
+	pci_write_config_dword(parent, PCI_BASE_ADDRESS_4, 0);
+
+	for (i = 2000; i > 0 ; i--) {
+		dirty = ls2k_bmc_bar0_addr_is_set(parent);
+		if (!dirty)
+			break;
+		mdelay(1);
+	};
+
+	if (i == 0)
+		dev_warn(priv->dev, "The PCI Bar is not cleared.\n");
+
+	for (i = 0; i < ARRAY_SIZE(index); i++)
+		pci_write_config_dword(parent, index[i], priv->pci_data.data[i]);
+
+	pci_write_config_dword(parent, 0x80c, priv->pci_data.d80c);
+	pci_write_config_dword(parent, 0x71c, priv->pci_data.d71c);
+
+	/* Check if the pcie is connected */
+	ready = ls2k_bmc_check_pcie_connected(parent, priv);
+	if (!ready)
+		return ready;
+
+	dev_dbg(priv->dev, "The PCIE recovered done.\n");
+
+	/* Waiting for u-boot ddr config ready */
+	mdelay(jiffies_to_msecs(LS2K_BMC_RESET_WAIT));
+	ready = ls2k_bmc_bar0_addr_is_set(parent);
+	if (!ready)
+		return ready;
+
+	for (i = 0; i < ARRAY_SIZE(cindex); i++)
+		pci_write_config_dword(pdev, cindex[i], priv->pci_data.cdata[i]);
+
+	return 0;
+}
+
+static void ls2k_bmc_events_fn(struct work_struct *work)
+{
+	struct ls2k_bmc_pdata *priv = container_of(work, struct ls2k_bmc_pdata, reset_work);
+
+	/*
+	 * The pcie is lost when the BMC resets,
+	 * at which point access to the pcie from other CPUs
+	 * is suspended to prevent a crash.
+	 */
+	stop_machine(ls2k_bmc_recover_pci_data, priv, NULL);
+
+#ifdef CONFIG_VT
+	/* Re-push the display due to previous pcie loss. */
+	set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
+#endif
+
+	dev_info(priv->dev, "Loongson-2K BMC recovered done.\n");
+}
+
+static irqreturn_t ls2k_bmc_interrupt(int irq, void *arg)
+{
+	struct ls2k_bmc_pdata *priv = arg;
+
+	if (system_state != SYSTEM_RUNNING)
+		return IRQ_HANDLED;
+
+	/* Skip interrupt in LS2K_BMC_RESET_DELAY */
+	if (time_after(jiffies, priv->reset_time + LS2K_BMC_RESET_DELAY))
+		schedule_work(&priv->reset_work);
+
+	priv->reset_time = jiffies;
+
+	return IRQ_HANDLED;
+}
+
+#define BMC_RESET_GPIO			14
+#define LOONGSON_GPIO_REG_BASE		0x1fe00500
+#define LOONGSON_GPIO_REG_SIZE		0x18
+#define LOONGSON_GPIO_OEN		0x0
+#define LOONGSON_GPIO_FUNC		0x4
+#define LOONGSON_GPIO_INTPOL		0x10
+#define LOONGSON_GPIO_INTEN		0x14
+
+/* The gpio interrupt is a watchdog interrupt that is triggered when the BMC resets. */
+static int ls2k_bmc_gpio_reset_handler(struct ls2k_bmc_pdata *priv)
+{
+	int gsi = 16 + (BMC_RESET_GPIO & 7);
+	void __iomem *gpio_base;
+	int irq, ret = 0;
+
+	/* Since Loongson-3A hardware does not support GPIO interrupt cascade,
+	 * chip->gpio_to_irq() cannot be implemented,
+	 * here acpi_register_gsi() is used to get gpio irq.
+	 */
+	irq = acpi_register_gsi(NULL, gsi, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW);
+	if (irq < 0)
+		return irq;
+
+	gpio_base = ioremap(LOONGSON_GPIO_REG_BASE, LOONGSON_GPIO_REG_SIZE);
+	if (!gpio_base) {
+		ret = PTR_ERR(gpio_base);
+		goto acpi_failed;
+	}
+
+	writel(readl(gpio_base + LOONGSON_GPIO_OEN) | BIT(BMC_RESET_GPIO),
+	       gpio_base + LOONGSON_GPIO_OEN);
+	writel(readl(gpio_base + LOONGSON_GPIO_FUNC) & ~BIT(BMC_RESET_GPIO),
+	       gpio_base + LOONGSON_GPIO_FUNC);
+	writel(readl(gpio_base + LOONGSON_GPIO_INTPOL) & ~BIT(BMC_RESET_GPIO),
+	       gpio_base + LOONGSON_GPIO_INTPOL);
+	writel(readl(gpio_base + LOONGSON_GPIO_INTEN) | BIT(BMC_RESET_GPIO),
+	       gpio_base + LOONGSON_GPIO_INTEN);
+
+	ret = devm_request_irq(priv->dev, irq, ls2k_bmc_interrupt,
+			       IRQF_SHARED | IRQF_TRIGGER_FALLING, "ls2kbmc gpio", priv);
+	if (ret)
+		dev_err(priv->dev, "ls2kbmc gpio request_irq(%d) failed\n", irq);
+
+	iounmap(gpio_base);
+
+acpi_failed:
+	acpi_unregister_gsi(gsi);
+
+	return ret;
+}
+
+static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
+{
+	struct pci_dev *parent = pdev->bus->self;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(index); i++)
+		pci_read_config_dword(parent, index[i], &priv->pci_data.data[i]);
+
+	for (i = 0; i < ARRAY_SIZE(cindex); i++)
+		pci_read_config_dword(pdev, cindex[i], &priv->pci_data.cdata[i]);
+
+	pci_read_config_dword(parent, 0x80c, &priv->pci_data.d80c);
+	priv->pci_data.d80c = (priv->pci_data.d80c & ~(3 << 17)) | BIT(17);
+
+	pci_read_config_dword(parent, 0x71c, &priv->pci_data.d71c);
+	priv->pci_data.d71c |= BIT(26);
+}
+
+static int ls2k_bmc_pdata_initial(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
+{
+	int ret;
+
+	ls2k_bmc_save_pci_data(pdev, priv);
+
+	INIT_WORK(&priv->reset_work, ls2k_bmc_events_fn);
+
+	ret = devm_request_irq(&pdev->dev, pdev->irq, ls2k_bmc_interrupt,
+			       IRQF_SHARED | IRQF_TRIGGER_FALLING, "ls2kbmc pcie", priv);
+	if (ret) {
+		dev_err(priv->dev, "ls2kbmc pcie request_irq(%d) failed\n", pdev->irq);
+		return ret;
+	}
+
+	return ls2k_bmc_gpio_reset_handler(priv);
+}
+
 /*
  * Currently the Loongson-2K0500 BMC hardware does not have an i2c interface to
  * adapt to the resolution.
@@ -101,12 +330,25 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	int ret = 0;
 	resource_size_t base;
+	struct ls2k_bmc_pdata *priv;
 	struct simplefb_platform_data pd;
 
 	ret = pci_enable_device(dev);
 	if (ret)
 		return ret;
 
+	priv = devm_kzalloc(&dev->dev, sizeof(*priv), GFP_KERNEL);
+	if (IS_ERR(priv)) {
+		ret = -ENOMEM;
+		goto disable_pci;
+	}
+
+	priv->dev = &dev->dev;
+
+	ret = ls2k_bmc_pdata_initial(dev, priv);
+	if (ret)
+		goto disable_pci;
+
 	ret = ls2k_bmc_get_video_mode(dev, &pd);
 	if (ret)
 		goto disable_pci;
-- 
2.47.1

Re: [PATCH v2 2/3] ipmi: Add Loongson-2K BMC support

[Corey Minyard]

On Thu, May 15, 2025 at 10:32:25AM +0800, Binbin Zhou wrote:
> This patch adds Loongson-2K BMC IPMI support.
> 
> According to the existing design, we use software simulation to
> implement the KCS interface registers: Stauts/Command/Data_Out/Data_In.

This is a strange way to do this.  My preference would be to have a
separate driver for this and not put it under the ipmi_si driver.
But it's annoyingly close and it would duplicate a lot of ipmi_si_intf.c
Anyway, I think I'm ok with this basic design.  But there are problems.

> 
> Also since both host side and BMC side read and write kcs status, I use
> fifo pointer to ensure data consistency.

I assume this fifo pointer is part of the interface hardware or the
implementation on the other side of the interface.

> 
> Therefore I made the whole IPMI driver independent.

What do you mean by this statement?

More comments inline.

> 
> Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  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 | 250 +++++++++++++++++++++++++++++++
>  4 files changed, 261 insertions(+)
>  create mode 100644 drivers/char/ipmi/ipmi_si_ls2k.c
> 
> diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
> index e0944547c9d0..5eb3494f5f39 100644
> --- a/drivers/char/ipmi/Makefile
> +++ b/drivers/char/ipmi/Makefile
> @@ -8,6 +8,7 @@ ipmi_si-y := ipmi_si_intf.o ipmi_kcs_sm.o ipmi_smic_sm.o ipmi_bt_sm.o \
>  	ipmi_si_mem_io.o
>  ipmi_si-$(CONFIG_HAS_IOPORT) += ipmi_si_port_io.o
>  ipmi_si-$(CONFIG_PCI) += ipmi_si_pci.o
> +ipmi_si-$(CONFIG_LOONGARCH) += ipmi_si_ls2k.o

Shouldn't this be dependent on MFD_LS2K_BMC?  It appears you can disable
that and still have CONFIG_LOONGARCH enabled.

And this MFD can have multiple things hanging off of it, wouldn't you
want to make the individual drivers their own CONFIG items?

>  ipmi_si-$(CONFIG_PARISC) += ipmi_si_parisc.o
>  
>  obj-$(CONFIG_IPMI_HANDLER) += ipmi_msghandler.o
> diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h
> index a7ead2a4c753..71f1d4e1272c 100644
> --- a/drivers/char/ipmi/ipmi_si.h
> +++ b/drivers/char/ipmi/ipmi_si.h
> @@ -93,6 +93,13 @@ void ipmi_si_pci_shutdown(void);
>  static inline void ipmi_si_pci_init(void) { }
>  static inline void ipmi_si_pci_shutdown(void) { }
>  #endif
> +#ifdef CONFIG_LOONGARCH
> +void ipmi_si_ls2k_init(void);
> +void ipmi_si_ls2k_shutdown(void);
> +#else
> +static inline void ipmi_si_ls2k_init(void) { }
> +static inline void ipmi_si_ls2k_shutdown(void) { }
> +#endif

I'm not excited about this, but there is history, I guess.

Same comment as the Makefile on CONFIG_LOONGARCH.

>  #ifdef CONFIG_PARISC
>  void ipmi_si_parisc_init(void);
>  void ipmi_si_parisc_shutdown(void);
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 12b0b77eb1cc..323da77698ea 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -2107,6 +2107,7 @@ static int __init init_ipmi_si(void)
>  
>  	ipmi_si_pci_init();
>  
> +	ipmi_si_ls2k_init();
>  	ipmi_si_parisc_init();
>  
>  	/* We prefer devices with interrupts, but in the case of a machine
> @@ -2288,6 +2289,8 @@ static void cleanup_ipmi_si(void)
>  
>  	ipmi_si_pci_shutdown();
>  
> +	ipmi_si_ls2k_shutdown();
> +
>  	ipmi_si_parisc_shutdown();
>  
>  	ipmi_si_platform_shutdown();
> diff --git a/drivers/char/ipmi/ipmi_si_ls2k.c b/drivers/char/ipmi/ipmi_si_ls2k.c
> new file mode 100644
> index 000000000000..cb31bb989fca
> --- /dev/null
> +++ b/drivers/char/ipmi/ipmi_si_ls2k.c
> @@ -0,0 +1,250 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for Loongson-2K BMC IPMI
> + *
> + * Copyright (C) 2024 Loongson Technology Corporation Limited.
> + *
> + * Originally written by Chong Qiao <qiaochong@loongson.cn>
> + * Rewritten for mainline by Binbin Zhou <zhoubinbin@loongson.cn>
> + */
> +
> +#include <linux/ioport.h>
> +#include <linux/module.h>
> +#include <linux/types.h>
> +
> +#include "ipmi_si.h"
> +
> +#define LS2K_KCS_STS_OBF	BIT(0)
> +#define LS2K_KCS_STS_IBF	BIT(1)
> +#define LS2K_KCS_STS_SMS_ATN	BIT(2)
> +#define LS2K_KCS_STS_CMD	BIT(3)
> +
> +#define LS2K_KCS_DATA_MASK	(LS2K_KCS_STS_OBF | LS2K_KCS_STS_IBF | LS2K_KCS_STS_CMD)
> +
> +/* Read and write fifo pointers for data consistency. */
> +struct ls2k_fifo_flag {
> +	u8 ibfh;
> +	u8 ibft;
> +	u8 obfh;
> +	u8 obft;
> +};
> +
> +struct ls2k_kcs_reg {
> +	u8 status;
> +	u8 data_out;
> +	s16 data_in;
> +	s16 cmd;
> +};
> +
> +struct ls2k_kcs_data {
> +	struct ls2k_fifo_flag fifo;
> +	struct ls2k_kcs_reg reg;
> +	u8 cmd_data;
> +	u8 version;
> +	u32 write_req;
> +	u32 write_ack;
> +	u32 reserved[2];
> +};

The above appears to be a memory overlay for registers.  But you aren't
using readb/writeb and associated functions to read/write it.  That is
not the right way to do things.  Please read
Documentation/driver-api/device-io.rst

> +
> +static void ls2k_set_obf(struct ls2k_kcs_data *ik, u8 sts)
> +{
> +	ik->reg.status = (ik->reg.status & ~LS2K_KCS_STS_OBF) | (sts & BIT(0));
> +}
> +
> +static void ls2k_set_ibf(struct ls2k_kcs_data *ik, u8 sts)
> +{
> +	ik->reg.status = (ik->reg.status & ~LS2K_KCS_STS_IBF) | ((sts & BIT(0)) << 1);
> +}
> +
> +static u8 ls2k_get_ibf(struct ls2k_kcs_data *ik)
> +{
> +	return (ik->reg.status >> 1) & BIT(0);
> +}
> +
> +static unsigned char intf_sim_inb_v0(struct ls2k_kcs_data *ik,
> +				     unsigned int offset)
> +{
> +	u32 inb = 0;
> +
> +	switch (offset & BIT(0)) {
> +	case 0:
> +		inb = ik->reg.data_out;
> +		ls2k_set_obf(ik, 0);
> +		break;
> +	case 1:
> +		inb = ik->reg.status;
> +		break;
> +	}
> +
> +	return inb;
> +}
> +
> +static unsigned char intf_sim_inb_v1(struct ls2k_kcs_data *ik,
> +				     unsigned int offset)
> +{
> +	u32 inb = 0;
> +	int cmd;
> +	bool obf, ibf;
> +
> +	obf = ik->fifo.obfh != ik->fifo.obft;
> +	ibf = ik->fifo.ibfh != ik->fifo.ibft;
> +	cmd = ik->cmd_data;
> +
> +	switch (offset & BIT(0)) {
> +	case 0:
> +		inb = ik->reg.data_out;
> +		ik->fifo.obft = ik->fifo.obfh;
> +		break;
> +	case 1:
> +		inb = ik->reg.status & ~LS2K_KCS_DATA_MASK;
> +		inb |= obf | (ibf << 1) | (cmd << 3);
> +		break;
> +	}
> +
> +	return inb;
> +}
> +
> +static unsigned char ls2k_mem_inb(const struct si_sm_io *io,
> +				  unsigned int offset)
> +{
> +	struct ls2k_kcs_data *ik = io->addr;
> +	int inb = 0;
> +
> +	if (ik->version == 0)
> +		inb = intf_sim_inb_v0(ik, offset);
> +	else if (ik->version == 1)
> +		inb = intf_sim_inb_v1(ik, offset);
> +
> +	return inb;
> +}
> +
> +static void intf_sim_outb_v0(struct ls2k_kcs_data *ik, unsigned int offset,
> +			     unsigned char val)
> +{
> +	if (ls2k_get_ibf(ik))
> +		return;
> +
> +	switch (offset & BIT(0)) {
> +	case 0:
> +		ik->reg.data_in = val;
> +		ik->reg.status &= ~LS2K_KCS_STS_CMD;
> +		break;
> +
> +	case 1:
> +		ik->reg.cmd = val;
> +		ik->reg.status |= LS2K_KCS_STS_CMD;
> +		break;
> +	}
> +
> +	ls2k_set_ibf(ik, 1);
> +	ik->write_req++;
> +}
> +
> +static void intf_sim_outb_v1(struct ls2k_kcs_data *ik, unsigned int offset,
> +			     unsigned char val)
> +{
> +	if (ik->fifo.ibfh != ik->fifo.ibft)
> +		return;
> +
> +	switch (offset & BIT(0)) {
> +	case 0:
> +		ik->reg.data_in = val;
> +		ik->cmd_data = 0;
> +		break;
> +
> +	case 1:
> +		ik->reg.cmd = val;
> +		ik->cmd_data = 1;
> +		break;
> +	}
> +
> +	ik->fifo.ibfh = !ik->fifo.ibft;
> +	ik->write_req++;
> +}
> +
> +static void ls2k_mem_outb(const struct si_sm_io *io, unsigned int offset,
> +			  unsigned char val)
> +{
> +	struct ls2k_kcs_data *ik = io->addr;
> +
> +	if (ik->version == 0)
> +		intf_sim_outb_v0(ik, offset, val);
> +	else if (ik->version == 1)
> +		intf_sim_outb_v1(ik, offset, val);
> +}
> +
> +static void ls2k_mem_cleanup(struct si_sm_io *io)
> +{
> +	if (io->addr)
> +		iounmap(io->addr);
> +}
> +
> +static int ipmi_ls2k_sim_setup(struct si_sm_io *io)
> +{
> +	io->addr = ioremap(io->addr_data, io->regspacing);
> +	if (!io->addr)
> +		return -EIO;
> +
> +	io->inputb = ls2k_mem_inb;
> +	io->outputb = ls2k_mem_outb;
> +	io->io_cleanup = ls2k_mem_cleanup;
> +
> +	return 0;
> +}
> +
> +static int ipmi_ls2k_probe(struct platform_device *pdev)
> +{
> +	struct si_sm_io io;
> +
> +	dev_info(&pdev->dev, "probing via ls2k platform");
> +	memset(&io, 0, sizeof(io));
> +
> +	io.addr_source	= SI_PLATFORM;
> +	io.si_type	= SI_KCS;

si_type has been reworked recently, the linux next tree has the changes.
I'll need this modified to work with the linux next changes.

> +	io.addr_space	= IPMI_MEM_ADDR_SPACE;
> +	io.io_setup	= ipmi_ls2k_sim_setup;
> +	io.addr_data	= pdev->resource[0].start;
> +	io.regspacing	= pdev->resource[0].end - pdev->resource[0].start + 1;
> +	io.regsize	= DEFAULT_REGSIZE;
> +	io.regshift	= 0;

The above items, except for io_setup,  don't have much meaning for your
device; there's not much need to set them, and there's no need to
initialize things to zero.  They are for ipmi_si_port and ipmi_si_mem.

> +	io.dev		= &pdev->dev;
> +	io.irq		= 0;
> +	if (io.irq)
> +		io.irq_setup = ipmi_std_irq_setup;

Just remove the irq thing, don't set it to zero and then check it.

> +
> +	dev_info(&pdev->dev, "%pR regsize %d spacing %d irq %d\n",
> +		 &pdev->resource[0], io.regsize, io.regspacing, io.irq);
> +
> +	return ipmi_si_add_smi(&io);
> +}
> +
> +static void ipmi_ls2k_remove(struct platform_device *pdev)
> +{
> +	ipmi_si_remove_by_dev(&pdev->dev);
> +}
> +
> +struct platform_driver ipmi_ls2k_platform_driver = {
> +	.driver = {
> +		.name = "ls2k-ipmi-si",
> +	},
> +	.probe	= ipmi_ls2k_probe,
> +	.remove	= ipmi_ls2k_remove,
> +};
> +
> +static bool platform_registered;
> +void ipmi_si_ls2k_init(void)
> +{
> +	int rv;
> +
> +	rv = platform_driver_register(&ipmi_ls2k_platform_driver);
> +	if (rv)
> +		pr_err("Unable to register driver: %d\n", rv);

That's far to vague to be useful.

> +	else
> +		platform_registered = true;
> +}
> +
> +void ipmi_si_ls2k_shutdown(void)
> +{
> +	if (platform_registered)
> +		platform_driver_unregister(&ipmi_ls2k_platform_driver);
> +}
> -- 
> 2.47.1
> 

Re: [PATCH v2 2/3] ipmi: Add Loongson-2K BMC support

[Binbin Zhou]

Hi Corey:

Thanks so much for the detailed review.

On Fri, May 16, 2025 at 8:59 AM Corey Minyard <corey@minyard.net> wrote:
>
> On Thu, May 15, 2025 at 10:32:25AM +0800, Binbin Zhou wrote:
> > This patch adds Loongson-2K BMC IPMI support.
> >
> > According to the existing design, we use software simulation to
> > implement the KCS interface registers: Stauts/Command/Data_Out/Data_In.
>
> This is a strange way to do this.  My preference would be to have a
> separate driver for this and not put it under the ipmi_si driver.
> But it's annoyingly close and it would duplicate a lot of ipmi_si_intf.c
> Anyway, I think I'm ok with this basic design.  But there are problems.
>
> >
> > Also since both host side and BMC side read and write kcs status, I use
> > fifo pointer to ensure data consistency.
>
> I assume this fifo pointer is part of the interface hardware or the
> implementation on the other side of the interface.
>
> >
> > Therefore I made the whole IPMI driver independent.
>
> What do you mean by this statement?
>
> More comments inline.
>
> >
> > Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> > Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > ---
> >  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 | 250 +++++++++++++++++++++++++++++++
> >  4 files changed, 261 insertions(+)
> >  create mode 100644 drivers/char/ipmi/ipmi_si_ls2k.c
> >
> > diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
> > index e0944547c9d0..5eb3494f5f39 100644
> > --- a/drivers/char/ipmi/Makefile
> > +++ b/drivers/char/ipmi/Makefile
> > @@ -8,6 +8,7 @@ ipmi_si-y := ipmi_si_intf.o ipmi_kcs_sm.o ipmi_smic_sm.o ipmi_bt_sm.o \
> >       ipmi_si_mem_io.o
> >  ipmi_si-$(CONFIG_HAS_IOPORT) += ipmi_si_port_io.o
> >  ipmi_si-$(CONFIG_PCI) += ipmi_si_pci.o
> > +ipmi_si-$(CONFIG_LOONGARCH) += ipmi_si_ls2k.o
>
> Shouldn't this be dependent on MFD_LS2K_BMC?  It appears you can disable
> that and still have CONFIG_LOONGARCH enabled.

Indeed, it should rely on MFD_LS2K_BMC.
>
> And this MFD can have multiple things hanging off of it, wouldn't you
> want to make the individual drivers their own CONFIG items?
>
> >  ipmi_si-$(CONFIG_PARISC) += ipmi_si_parisc.o
> >
> >  obj-$(CONFIG_IPMI_HANDLER) += ipmi_msghandler.o
> > diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h
> > index a7ead2a4c753..71f1d4e1272c 100644
> > --- a/drivers/char/ipmi/ipmi_si.h
> > +++ b/drivers/char/ipmi/ipmi_si.h
> > @@ -93,6 +93,13 @@ void ipmi_si_pci_shutdown(void);
> >  static inline void ipmi_si_pci_init(void) { }
> >  static inline void ipmi_si_pci_shutdown(void) { }
> >  #endif
> > +#ifdef CONFIG_LOONGARCH
> > +void ipmi_si_ls2k_init(void);
> > +void ipmi_si_ls2k_shutdown(void);
> > +#else
> > +static inline void ipmi_si_ls2k_init(void) { }
> > +static inline void ipmi_si_ls2k_shutdown(void) { }
> > +#endif
>
> I'm not excited about this, but there is history, I guess.
>
> Same comment as the Makefile on CONFIG_LOONGARCH.

It should be MFD_LS2K_BMC.
>
> >  #ifdef CONFIG_PARISC
> >  void ipmi_si_parisc_init(void);
> >  void ipmi_si_parisc_shutdown(void);
> > diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> > index 12b0b77eb1cc..323da77698ea 100644
> > --- a/drivers/char/ipmi/ipmi_si_intf.c
> > +++ b/drivers/char/ipmi/ipmi_si_intf.c
> > @@ -2107,6 +2107,7 @@ static int __init init_ipmi_si(void)
> >
> >       ipmi_si_pci_init();
> >
> > +     ipmi_si_ls2k_init();
> >       ipmi_si_parisc_init();
> >
> >       /* We prefer devices with interrupts, but in the case of a machine
> > @@ -2288,6 +2289,8 @@ static void cleanup_ipmi_si(void)
> >
> >       ipmi_si_pci_shutdown();
> >
> > +     ipmi_si_ls2k_shutdown();
> > +
> >       ipmi_si_parisc_shutdown();
> >
> >       ipmi_si_platform_shutdown();
> > diff --git a/drivers/char/ipmi/ipmi_si_ls2k.c b/drivers/char/ipmi/ipmi_si_ls2k.c
> > new file mode 100644
> > index 000000000000..cb31bb989fca
> > --- /dev/null
> > +++ b/drivers/char/ipmi/ipmi_si_ls2k.c
> > @@ -0,0 +1,250 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Driver for Loongson-2K BMC IPMI
> > + *
> > + * Copyright (C) 2024 Loongson Technology Corporation Limited.
> > + *
> > + * Originally written by Chong Qiao <qiaochong@loongson.cn>
> > + * Rewritten for mainline by Binbin Zhou <zhoubinbin@loongson.cn>
> > + */
> > +
> > +#include <linux/ioport.h>
> > +#include <linux/module.h>
> > +#include <linux/types.h>
> > +
> > +#include "ipmi_si.h"
> > +
> > +#define LS2K_KCS_STS_OBF     BIT(0)
> > +#define LS2K_KCS_STS_IBF     BIT(1)
> > +#define LS2K_KCS_STS_SMS_ATN BIT(2)
> > +#define LS2K_KCS_STS_CMD     BIT(3)
> > +
> > +#define LS2K_KCS_DATA_MASK   (LS2K_KCS_STS_OBF | LS2K_KCS_STS_IBF | LS2K_KCS_STS_CMD)
> > +
> > +/* Read and write fifo pointers for data consistency. */
> > +struct ls2k_fifo_flag {
> > +     u8 ibfh;
> > +     u8 ibft;
> > +     u8 obfh;
> > +     u8 obft;
> > +};
> > +
> > +struct ls2k_kcs_reg {
> > +     u8 status;
> > +     u8 data_out;
> > +     s16 data_in;
> > +     s16 cmd;
> > +};
> > +
> > +struct ls2k_kcs_data {
> > +     struct ls2k_fifo_flag fifo;
> > +     struct ls2k_kcs_reg reg;
> > +     u8 cmd_data;
> > +     u8 version;
> > +     u32 write_req;
> > +     u32 write_ack;
> > +     u32 reserved[2];
> > +};
>
> The above appears to be a memory overlay for registers.  But you aren't
> using readb/writeb and associated functions to read/write it.  That is
> not the right way to do things.  Please read
> Documentation/driver-api/device-io.rst

Ok, I'll refactor this part of the code to redefine each struct item
into the appropriate macro and access it via readx()/writex().
>
> > +
> > +static void ls2k_set_obf(struct ls2k_kcs_data *ik, u8 sts)
> > +{
> > +     ik->reg.status = (ik->reg.status & ~LS2K_KCS_STS_OBF) | (sts & BIT(0));
> > +}
> > +
> > +static void ls2k_set_ibf(struct ls2k_kcs_data *ik, u8 sts)
> > +{
> > +     ik->reg.status = (ik->reg.status & ~LS2K_KCS_STS_IBF) | ((sts & BIT(0)) << 1);
> > +}
> > +
> > +static u8 ls2k_get_ibf(struct ls2k_kcs_data *ik)
> > +{
> > +     return (ik->reg.status >> 1) & BIT(0);
> > +}
> > +
> > +static unsigned char intf_sim_inb_v0(struct ls2k_kcs_data *ik,
> > +                                  unsigned int offset)
> > +{
> > +     u32 inb = 0;
> > +
> > +     switch (offset & BIT(0)) {
> > +     case 0:
> > +             inb = ik->reg.data_out;
> > +             ls2k_set_obf(ik, 0);
> > +             break;
> > +     case 1:
> > +             inb = ik->reg.status;
> > +             break;
> > +     }
> > +
> > +     return inb;
> > +}
> > +
> > +static unsigned char intf_sim_inb_v1(struct ls2k_kcs_data *ik,
> > +                                  unsigned int offset)
> > +{
> > +     u32 inb = 0;
> > +     int cmd;
> > +     bool obf, ibf;
> > +
> > +     obf = ik->fifo.obfh != ik->fifo.obft;
> > +     ibf = ik->fifo.ibfh != ik->fifo.ibft;
> > +     cmd = ik->cmd_data;
> > +
> > +     switch (offset & BIT(0)) {
> > +     case 0:
> > +             inb = ik->reg.data_out;
> > +             ik->fifo.obft = ik->fifo.obfh;
> > +             break;
> > +     case 1:
> > +             inb = ik->reg.status & ~LS2K_KCS_DATA_MASK;
> > +             inb |= obf | (ibf << 1) | (cmd << 3);
> > +             break;
> > +     }
> > +
> > +     return inb;
> > +}
> > +
> > +static unsigned char ls2k_mem_inb(const struct si_sm_io *io,
> > +                               unsigned int offset)
> > +{
> > +     struct ls2k_kcs_data *ik = io->addr;
> > +     int inb = 0;
> > +
> > +     if (ik->version == 0)
> > +             inb = intf_sim_inb_v0(ik, offset);
> > +     else if (ik->version == 1)
> > +             inb = intf_sim_inb_v1(ik, offset);
> > +
> > +     return inb;
> > +}
> > +
> > +static void intf_sim_outb_v0(struct ls2k_kcs_data *ik, unsigned int offset,
> > +                          unsigned char val)
> > +{
> > +     if (ls2k_get_ibf(ik))
> > +             return;
> > +
> > +     switch (offset & BIT(0)) {
> > +     case 0:
> > +             ik->reg.data_in = val;
> > +             ik->reg.status &= ~LS2K_KCS_STS_CMD;
> > +             break;
> > +
> > +     case 1:
> > +             ik->reg.cmd = val;
> > +             ik->reg.status |= LS2K_KCS_STS_CMD;
> > +             break;
> > +     }
> > +
> > +     ls2k_set_ibf(ik, 1);
> > +     ik->write_req++;
> > +}
> > +
> > +static void intf_sim_outb_v1(struct ls2k_kcs_data *ik, unsigned int offset,
> > +                          unsigned char val)
> > +{
> > +     if (ik->fifo.ibfh != ik->fifo.ibft)
> > +             return;
> > +
> > +     switch (offset & BIT(0)) {
> > +     case 0:
> > +             ik->reg.data_in = val;
> > +             ik->cmd_data = 0;
> > +             break;
> > +
> > +     case 1:
> > +             ik->reg.cmd = val;
> > +             ik->cmd_data = 1;
> > +             break;
> > +     }
> > +
> > +     ik->fifo.ibfh = !ik->fifo.ibft;
> > +     ik->write_req++;
> > +}
> > +
> > +static void ls2k_mem_outb(const struct si_sm_io *io, unsigned int offset,
> > +                       unsigned char val)
> > +{
> > +     struct ls2k_kcs_data *ik = io->addr;
> > +
> > +     if (ik->version == 0)
> > +             intf_sim_outb_v0(ik, offset, val);
> > +     else if (ik->version == 1)
> > +             intf_sim_outb_v1(ik, offset, val);
> > +}
> > +
> > +static void ls2k_mem_cleanup(struct si_sm_io *io)
> > +{
> > +     if (io->addr)
> > +             iounmap(io->addr);
> > +}
> > +
> > +static int ipmi_ls2k_sim_setup(struct si_sm_io *io)
> > +{
> > +     io->addr = ioremap(io->addr_data, io->regspacing);
> > +     if (!io->addr)
> > +             return -EIO;
> > +
> > +     io->inputb = ls2k_mem_inb;
> > +     io->outputb = ls2k_mem_outb;
> > +     io->io_cleanup = ls2k_mem_cleanup;
> > +
> > +     return 0;
> > +}
> > +
> > +static int ipmi_ls2k_probe(struct platform_device *pdev)
> > +{
> > +     struct si_sm_io io;
> > +
> > +     dev_info(&pdev->dev, "probing via ls2k platform");
> > +     memset(&io, 0, sizeof(io));
> > +
> > +     io.addr_source  = SI_PLATFORM;
> > +     io.si_type      = SI_KCS;
>
> si_type has been reworked recently, the linux next tree has the changes.
> I'll need this modified to work with the linux next changes.

OK, I will rebase my driver.
>
> > +     io.addr_space   = IPMI_MEM_ADDR_SPACE;
> > +     io.io_setup     = ipmi_ls2k_sim_setup;
> > +     io.addr_data    = pdev->resource[0].start;
> > +     io.regspacing   = pdev->resource[0].end - pdev->resource[0].start + 1;
> > +     io.regsize      = DEFAULT_REGSIZE;
> > +     io.regshift     = 0;
>
> The above items, except for io_setup,  don't have much meaning for your
> device; there's not much need to set them, and there's no need to
> initialize things to zero.  They are for ipmi_si_port and ipmi_si_mem.

The addr_data and regspacing seem to be needed in
ipmi_ls2k_sim_setup(), in any case, I'll reorganize it based on the
latest code.
>
> > +     io.dev          = &pdev->dev;
> > +     io.irq          = 0;
> > +     if (io.irq)
> > +             io.irq_setup = ipmi_std_irq_setup;
>
> Just remove the irq thing, don't set it to zero and then check it.

OK..
>
> > +
> > +     dev_info(&pdev->dev, "%pR regsize %d spacing %d irq %d\n",
> > +              &pdev->resource[0], io.regsize, io.regspacing, io.irq);
> > +
> > +     return ipmi_si_add_smi(&io);
> > +}
> > +
> > +static void ipmi_ls2k_remove(struct platform_device *pdev)
> > +{
> > +     ipmi_si_remove_by_dev(&pdev->dev);
> > +}
> > +
> > +struct platform_driver ipmi_ls2k_platform_driver = {
> > +     .driver = {
> > +             .name = "ls2k-ipmi-si",
> > +     },
> > +     .probe  = ipmi_ls2k_probe,
> > +     .remove = ipmi_ls2k_remove,
> > +};
> > +
> > +static bool platform_registered;
> > +void ipmi_si_ls2k_init(void)
> > +{
> > +     int rv;
> > +
> > +     rv = platform_driver_register(&ipmi_ls2k_platform_driver);
> > +     if (rv)
> > +             pr_err("Unable to register driver: %d\n", rv);
>
> That's far to vague to be useful.

OK, let's just drop it.
>
> > +     else
> > +             platform_registered = true;
> > +}
> > +
> > +void ipmi_si_ls2k_shutdown(void)
> > +{
> > +     if (platform_registered)
> > +             platform_driver_unregister(&ipmi_ls2k_platform_driver);
> > +}
> > --
> > 2.47.1
> >

--
Thanks.
Binbin

Re: [PATCH v2 1/3] mfd: ls2kbmc: Introduce Loongson-2K BMC MFD Core driver

[Lee Jones]

Just "core driver" in the subject line, rather than "MFD core driver".

> The Loongson-2K Board Management Controller provides an PCIe
> interface to the host to access the feature implemented in the BMC.
> 
> The BMC is assembled on a server similar to the server machine with
> Loongson-3C6000 CPUs. It supports multiple sub-devices like DRM.
> 
> Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  drivers/mfd/Kconfig       |  13 ++++
>  drivers/mfd/Makefile      |   2 +
>  drivers/mfd/ls2kbmc-mfd.c | 156 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 171 insertions(+)
>  create mode 100644 drivers/mfd/ls2kbmc-mfd.c
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 22b936310039..04e40085441d 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2422,5 +2422,18 @@ config MFD_UPBOARD_FPGA
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called upboard-fpga.
>  
> +config MFD_LS2K_BMC
> +	tristate "Loongson-2K Board Management Controller Support"
> +	depends on LOONGARCH
> +	default y if LOONGARCH
> +	select MFD_CORE
> +	help
> +	  Say yes here to add support for the Loongson-2K BMC
> +	  which is a Board Management Controller connected to the PCIe bus.
> +	  The device supports multiple sub-devices like DRM.
> +	  This driver provides common support for accessing the devices;
> +	  additional drivers must be enabled in order to use the
> +	  functionality of the BMC device.

This paragraph has some odd line breaks.  Please re-align.

>  endmenu
>  endif
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 948cbdf42a18..18960ea13b64 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -290,3 +290,5 @@ obj-$(CONFIG_MFD_RSMU_I2C)	+= rsmu_i2c.o rsmu_core.o
>  obj-$(CONFIG_MFD_RSMU_SPI)	+= rsmu_spi.o rsmu_core.o
>  
>  obj-$(CONFIG_MFD_UPBOARD_FPGA)	+= upboard-fpga.o
> +
> +obj-$(CONFIG_MFD_LS2K_BMC)	+= ls2kbmc-mfd.o
> diff --git a/drivers/mfd/ls2kbmc-mfd.c b/drivers/mfd/ls2kbmc-mfd.c
> new file mode 100644
> index 000000000000..b309f6132c24
> --- /dev/null
> +++ b/drivers/mfd/ls2kbmc-mfd.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Loongson-2K Board Management Controller (BMC) MFD Core Driver.

Remove the MFD part.  It's not a thing - we made it up.

> + * Copyright (C) 2024 Loongson Technology Corporation Limited.

No changes since 2024?

> + *
> + * Originally written by Chong Qiao <qiaochong@loongson.cn>
> + * Rewritten for mainline by Binbin Zhou <zhoubinbin@loongson.cn>

"Mainline"

Typically we just do:

Authors:
	Author One <one@corp.com>
	Author Two <two@corp.com>

> + */
> +
> +#include <linux/aperture.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/pci_ids.h>
> +#include <linux/platform_data/simplefb.h>
> +#include <linux/platform_device.h>
> +
> +#define LS2K_DISPLAY_RES_START (SZ_16M + SZ_2M)
> +#define LS2K_IPMI_RES_SIZE	0x1c
> +#define LS2K_IPMI0_RES_START	(SZ_16M + 0xf00000)
> +#define LS2K_IPMI1_RES_START	(LS2K_IPMI0_RES_START + LS2K_IPMI_RES_SIZE)
> +#define LS2K_IPMI2_RES_START	(LS2K_IPMI1_RES_START + LS2K_IPMI_RES_SIZE)
> +#define LS2K_IPMI3_RES_START	(LS2K_IPMI2_RES_START + LS2K_IPMI_RES_SIZE)
> +#define LS2K_IPMI4_RES_START	(LS2K_IPMI3_RES_START + LS2K_IPMI_RES_SIZE)

Line them _all_ up please.  One more tab should do it.

> +static struct resource ls2k_display_resources[] = {
> +	DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
> +};
> +
> +static struct resource ls2k_ipmi0_resources[] = {
> +	DEFINE_RES_MEM_NAMED(LS2K_IPMI0_RES_START, LS2K_IPMI_RES_SIZE, "ipmi0-res"),
> +};
> +
> +static struct resource ls2k_ipmi1_resources[] = {
> +	DEFINE_RES_MEM_NAMED(LS2K_IPMI1_RES_START, LS2K_IPMI_RES_SIZE, "ipmi1-res"),
> +};
> +
> +static struct resource ls2k_ipmi2_resources[] = {
> +	DEFINE_RES_MEM_NAMED(LS2K_IPMI2_RES_START, LS2K_IPMI_RES_SIZE, "ipmi2-res"),
> +};
> +
> +static struct resource ls2k_ipmi3_resources[] = {
> +	DEFINE_RES_MEM_NAMED(LS2K_IPMI3_RES_START, LS2K_IPMI_RES_SIZE, "ipmi3-res"),
> +};
> +
> +static struct resource ls2k_ipmi4_resources[] = {
> +	DEFINE_RES_MEM_NAMED(LS2K_IPMI4_RES_START, LS2K_IPMI_RES_SIZE, "ipmi4-res"),
> +};
> +
> +static struct mfd_cell ls2k_bmc_cells[] = {
> +	MFD_CELL_RES("simple-framebuffer", ls2k_display_resources),
> +	MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi0_resources),
> +	MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi1_resources),
> +	MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi2_resources),
> +	MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi3_resources),
> +	MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
> +};
> +
> +/*
> + * Currently the Loongson-2K0500 BMC hardware does not have an i2c interface to

I2C

> + * adapt to the resolution.

Remove the line break here.

> + * We set the resolution by presetting "video=1280x1024-16@2M" to the bmc memory.

BMC

> + */
> +static int ls2k_bmc_get_video_mode(struct pci_dev *pdev, struct simplefb_platform_data *pd)
> +{
> +	char *mode;
> +	int depth, ret;
> +
> +	/* The pci mem bar last 16M is used to store the string. */

PCI

BAR's (maybe?)

> +	mode = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0) + SZ_16M, SZ_16M);
> +	if (!mode)
> +		return -ENOMEM;
> +
> +	/* env at last 16M's beginning, first env is "video=" */

This doesn't make sense to me - please reword.

> +	if (!strncmp(mode, "video=", 6))
> +		mode = mode + 6;
> +
> +	ret = kstrtoint(strsep(&mode, "x"), 10, &pd->width);
> +	if (ret)
> +		return ret;
> +
> +	ret = kstrtoint(strsep(&mode, "-"), 10, &pd->height);
> +	if (ret)
> +		return ret;
> +
> +	ret = kstrtoint(strsep(&mode, "@"), 10, &depth);
> +	if (ret)
> +		return ret;
> +
> +	pd->stride = pd->width * depth / 8;
> +	pd->format = depth == 32 ? "a8r8g8b8" : "r5g6b5";
> +
> +	return 0;
> +}

Surely there is a standard format / API for this already?



> +static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
> +{
> +	int ret = 0;

There is no need to pre-initialise this.

> +	resource_size_t base;
> +	struct simplefb_platform_data pd;

Reverse these please (reverse Christmas tree is preferred).
> +
> +	ret = pci_enable_device(dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = ls2k_bmc_get_video_mode(dev, &pd);
> +	if (ret)
> +		goto disable_pci;
> +
> +	ls2k_bmc_cells[0].platform_data = &pd;
> +	ls2k_bmc_cells[0].pdata_size = sizeof(pd);
> +	base = dev->resource[0].start + LS2K_DISPLAY_RES_START;
> +
> +	/* Remove conflicting efifb device */
> +	ret = aperture_remove_conflicting_devices(base, SZ_4M, "simple-framebuffer");
> +	if (ret) {
> +		dev_err(&dev->dev, "Remove firmware framebuffers failed: %d\n", ret);

"Failed to removed firmware framebuffers"

> +		goto disable_pci;
> +	}
> +
> +	return devm_mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
> +				    ls2k_bmc_cells, ARRAY_SIZE(ls2k_bmc_cells),
> +				    &dev->resource[0], 0, NULL);
> +
> +disable_pci:
> +	pci_disable_device(dev);
> +	return ret;
> +}
> +
> +static void ls2k_bmc_remove(struct pci_dev *dev)
> +{
> +	pci_disable_device(dev);
> +}
> +
> +static struct pci_device_id ls2k_bmc_devices[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x1a05) },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(pci, ls2k_bmc_devices);
> +
> +static struct pci_driver ls2k_bmc_driver = {
> +	.name = "ls2k-bmc",
> +	.id_table = ls2k_bmc_devices,
> +	.probe = ls2k_bmc_probe,
> +	.remove = ls2k_bmc_remove,
> +};
> +

Remove this line.

> +module_pci_driver(ls2k_bmc_driver);
> +
> +MODULE_DESCRIPTION("Loongson-2K BMC driver");
> +MODULE_AUTHOR("Loongson Technology Corporation Limited");
> +MODULE_LICENSE("GPL");
> -- 
> 2.47.1
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH v2 3/3] mfd: ls2kbmc: Add Loongson-2K BMC reset function support

[Lee Jones]

On Thu, 15 May 2025, Binbin Zhou wrote:

> Since the display is a sub-function of the Loongson-2K BMC, when the
> BMC reset, the entire BMC PCIe is disconnected, including the display
> which is interrupted.
> 
> Not only do you need to save/restore the relevant PCIe registers
> throughout the reset process, but you also need to re-push the display
> to the monitor at the end.
> 
> Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  drivers/mfd/ls2kbmc-mfd.c | 242 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 242 insertions(+)
> 
> diff --git a/drivers/mfd/ls2kbmc-mfd.c b/drivers/mfd/ls2kbmc-mfd.c
> index b309f6132c24..4d35a13b3da5 100644
> --- a/drivers/mfd/ls2kbmc-mfd.c
> +++ b/drivers/mfd/ls2kbmc-mfd.c
> @@ -9,8 +9,11 @@
>   */
>  
>  #include <linux/aperture.h>
> +#include <linux/delay.h>
>  #include <linux/errno.h>
>  #include <linux/init.h>
> +#include <linux/iopoll.h>
> +#include <linux/kbd_kern.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/core.h>
>  #include <linux/module.h>
> @@ -18,6 +21,8 @@
>  #include <linux/pci_ids.h>
>  #include <linux/platform_data/simplefb.h>
>  #include <linux/platform_device.h>
> +#include <linux/stop_machine.h>
> +#include <linux/vt_kern.h>
>  
>  #define LS2K_DISPLAY_RES_START (SZ_16M + SZ_2M)
>  #define LS2K_IPMI_RES_SIZE	0x1c
> @@ -27,6 +32,9 @@
>  #define LS2K_IPMI3_RES_START	(LS2K_IPMI2_RES_START + LS2K_IPMI_RES_SIZE)
>  #define LS2K_IPMI4_RES_START	(LS2K_IPMI3_RES_START + LS2K_IPMI_RES_SIZE)
>  
> +#define LS2K_BMC_RESET_DELAY	(60 * HZ)
> +#define LS2K_BMC_RESET_WAIT	(10 * HZ)
> +
>  static struct resource ls2k_display_resources[] = {
>  	DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
>  };
> @@ -60,6 +68,227 @@ static struct mfd_cell ls2k_bmc_cells[] = {
>  	MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
>  };
>  
> +static const u32 index[] = { 0x4, 0x10, 0x14, 0x18, 0x1c, 0x20, 0x24,
> +			     0x30, 0x3c, 0x54, 0x58, 0x78, 0x7c, 0x80 };
> +static const u32 cindex[] = { 0x4, 0x10, 0x3c };

What are these?  You need to define them or at least provide a comment
that describes what they are and how they're used.

> +struct ls2k_bmc_pci_data {
> +	u32 d80c;
> +	u32 d71c;
> +	u32 data[14];
> +	u32 cdata[3];

To the uninitiated, this is all gibberish.

Either change the nomenclature or provide commentary.

> +};
> +
> +struct ls2k_bmc_pdata {
> +	struct device *dev;
> +	struct work_struct reset_work;
> +	unsigned long reset_time;

This always appears to be jiffies - is it used elsewhere?

> +	struct ls2k_bmc_pci_data pci_data;
> +};
> +
> +static bool ls2k_bmc_bar0_addr_is_set(struct pci_dev *ppdev)
> +{
> +	u32 addr;
> +
> +	pci_read_config_dword(ppdev, PCI_BASE_ADDRESS_0, &addr);
> +	addr &= PCI_BASE_ADDRESS_MEM_MASK;
> +
> +	return addr ? true : false;
> +}
> +
> +static bool ls2k_bmc_check_pcie_connected(struct pci_dev *parent,
> +					  struct ls2k_bmc_pdata *priv)
> +{
> +	void __iomem *mmio;
> +	int sts, ret;
> +
> +	mmio = pci_iomap(parent, 0, 0x100);

Why 0x100?  This should be defined.

> +	if (!mmio)
> +		return false;
> +
> +	writel(readl(mmio) | 0x8, mmio);
> +
> +	ret = readl_poll_timeout_atomic(mmio + 0xc, sts, (sts & 0x11) == 0x11,
> +					1000, 1000000);
> +	if (ret) {

All of the magic numbers in this function should be defined.

> +		pci_iounmap(parent, mmio);
> +		dev_err(priv->dev, "PCIE train failed status=0x%x\n", sts);
> +		return false;
> +	}
> +
> +	pci_iounmap(parent, mmio);
> +	return true;
> +}
> +
> +static int ls2k_bmc_recover_pci_data(void *data)
> +{
> +	struct ls2k_bmc_pdata *priv = data;
> +	struct pci_dev *pdev = to_pci_dev(priv->dev);
> +	struct pci_dev *parent = pdev->bus->self;
> +	bool ready, dirty;
> +	u32 i;
> +
> +	pci_write_config_dword(parent, PCI_BASE_ADDRESS_2, 0);
> +	pci_write_config_dword(parent, PCI_BASE_ADDRESS_3, 0);
> +	pci_write_config_dword(parent, PCI_BASE_ADDRESS_4, 0);

You need to provide commentary throughout this driver!

> +
> +	for (i = 2000; i > 0 ; i--) {

Why 2000?  Please define and explain it.

> +		dirty = ls2k_bmc_bar0_addr_is_set(parent);

Since these are bools, you can put the function in an if() statement.

> +		if (!dirty)
> +			break;
> +		mdelay(1);

Why?  What are we waiting for?

> +	};
> +
> +	if (i == 0)
> +		dev_warn(priv->dev, "The PCI Bar is not cleared.\n");

But it's okay to continue?  Why?

> +	for (i = 0; i < ARRAY_SIZE(index); i++)
> +		pci_write_config_dword(parent, index[i], priv->pci_data.data[i]);
> +
> +	pci_write_config_dword(parent, 0x80c, priv->pci_data.d80c);
> +	pci_write_config_dword(parent, 0x71c, priv->pci_data.d71c);
> +
> +	/* Check if the pcie is connected */

PCI-E

> +	ready = ls2k_bmc_check_pcie_connected(parent, priv);

As above.

> +	if (!ready)
> +		return ready;
> +
> +	dev_dbg(priv->dev, "The PCIE recovered done.\n");

How useful is this once development is complete?

> +	/* Waiting for u-boot ddr config ready */

Will it always be U-Boot?

Should be "U-Boot" and "DDR".

> +	mdelay(jiffies_to_msecs(LS2K_BMC_RESET_WAIT));

Why not define the value in ms already?

> +	ready = ls2k_bmc_bar0_addr_is_set(parent);

As above.

> +	if (!ready)
> +		return ready;
> +
> +	for (i = 0; i < ARRAY_SIZE(cindex); i++)
> +		pci_write_config_dword(pdev, cindex[i], priv->pci_data.cdata[i]);
> +
> +	return 0;
> +}
> +
> +static void ls2k_bmc_events_fn(struct work_struct *work)
> +{
> +	struct ls2k_bmc_pdata *priv = container_of(work, struct ls2k_bmc_pdata, reset_work);
> +
> +	/*
> +	 * The pcie is lost when the BMC resets,

Strange line-breaks.  Pick a limit (80-100, etc) and stick to it.

> +	 * at which point access to the pcie from other CPUs
> +	 * is suspended to prevent a crash.
> +	 */
> +	stop_machine(ls2k_bmc_recover_pci_data, priv, NULL);
> +
> +#ifdef CONFIG_VT

No avoidable #ifery in C-files please.

> +	/* Re-push the display due to previous pcie loss. */
> +	set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> +#endif
> +
> +	dev_info(priv->dev, "Loongson-2K BMC recovered done.\n");

Remove this line - we want clean bootlogs.

> +}
> +
> +static irqreturn_t ls2k_bmc_interrupt(int irq, void *arg)
> +{
> +	struct ls2k_bmc_pdata *priv = arg;
> +
> +	if (system_state != SYSTEM_RUNNING)
> +		return IRQ_HANDLED;
> +
> +	/* Skip interrupt in LS2K_BMC_RESET_DELAY */
> +	if (time_after(jiffies, priv->reset_time + LS2K_BMC_RESET_DELAY))
> +		schedule_work(&priv->reset_work);
> +
> +	priv->reset_time = jiffies;
> +
> +	return IRQ_HANDLED;
> +}
> +
> +#define BMC_RESET_GPIO			14
> +#define LOONGSON_GPIO_REG_BASE		0x1fe00500
> +#define LOONGSON_GPIO_REG_SIZE		0x18
> +#define LOONGSON_GPIO_OEN		0x0
> +#define LOONGSON_GPIO_FUNC		0x4
> +#define LOONGSON_GPIO_INTPOL		0x10
> +#define LOONGSON_GPIO_INTEN		0x14

Put these up with the reset of the defines.

> +/* The gpio interrupt is a watchdog interrupt that is triggered when the BMC resets. */
> +static int ls2k_bmc_gpio_reset_handler(struct ls2k_bmc_pdata *priv)

Why are we now doing GPIO things in here?  Why not a GPIO driver?

> +{
> +	int gsi = 16 + (BMC_RESET_GPIO & 7);
> +	void __iomem *gpio_base;
> +	int irq, ret = 0;
> +
> +	/* Since Loongson-3A hardware does not support GPIO interrupt cascade,
> +	 * chip->gpio_to_irq() cannot be implemented,
> +	 * here acpi_register_gsi() is used to get gpio irq.
> +	 */

Use proper multi-line commentary as per the Coding Style documentation.

> +	irq = acpi_register_gsi(NULL, gsi, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW);
> +	if (irq < 0)
> +		return irq;
> +
> +	gpio_base = ioremap(LOONGSON_GPIO_REG_BASE, LOONGSON_GPIO_REG_SIZE);
> +	if (!gpio_base) {
> +		ret = PTR_ERR(gpio_base);
> +		goto acpi_failed;
> +	}
> +
> +	writel(readl(gpio_base + LOONGSON_GPIO_OEN) | BIT(BMC_RESET_GPIO),
> +	       gpio_base + LOONGSON_GPIO_OEN);
> +	writel(readl(gpio_base + LOONGSON_GPIO_FUNC) & ~BIT(BMC_RESET_GPIO),
> +	       gpio_base + LOONGSON_GPIO_FUNC);
> +	writel(readl(gpio_base + LOONGSON_GPIO_INTPOL) & ~BIT(BMC_RESET_GPIO),
> +	       gpio_base + LOONGSON_GPIO_INTPOL);
> +	writel(readl(gpio_base + LOONGSON_GPIO_INTEN) | BIT(BMC_RESET_GPIO),
> +	       gpio_base + LOONGSON_GPIO_INTEN);
> +
> +	ret = devm_request_irq(priv->dev, irq, ls2k_bmc_interrupt,
> +			       IRQF_SHARED | IRQF_TRIGGER_FALLING, "ls2kbmc gpio", priv);
> +	if (ret)
> +		dev_err(priv->dev, "ls2kbmc gpio request_irq(%d) failed\n", irq);

"LS2KBMC" and "GPIO", etc.

> +
> +	iounmap(gpio_base);
> +
> +acpi_failed:
> +	acpi_unregister_gsi(gsi);
> +
> +	return ret;
> +}
> +
> +static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> +{
> +	struct pci_dev *parent = pdev->bus->self;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(index); i++)
> +		pci_read_config_dword(parent, index[i], &priv->pci_data.data[i]);
> +
> +	for (i = 0; i < ARRAY_SIZE(cindex); i++)
> +		pci_read_config_dword(pdev, cindex[i], &priv->pci_data.cdata[i]);
> +
> +	pci_read_config_dword(parent, 0x80c, &priv->pci_data.d80c);
> +	priv->pci_data.d80c = (priv->pci_data.d80c & ~(3 << 17)) | BIT(17);
> +
> +	pci_read_config_dword(parent, 0x71c, &priv->pci_data.d71c);
> +	priv->pci_data.d71c |= BIT(26);

No magic numbers please.  Define them all.

> +}
> +
> +static int ls2k_bmc_pdata_initial(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> +{
> +	int ret;
> +
> +	ls2k_bmc_save_pci_data(pdev, priv);
> +
> +	INIT_WORK(&priv->reset_work, ls2k_bmc_events_fn);
> +
> +	ret = devm_request_irq(&pdev->dev, pdev->irq, ls2k_bmc_interrupt,
> +			       IRQF_SHARED | IRQF_TRIGGER_FALLING, "ls2kbmc pcie", priv);
> +	if (ret) {
> +		dev_err(priv->dev, "ls2kbmc pcie request_irq(%d) failed\n", pdev->irq);
> +		return ret;
> +	}
> +
> +	return ls2k_bmc_gpio_reset_handler(priv);
> +}
> +
>  /*
>   * Currently the Loongson-2K0500 BMC hardware does not have an i2c interface to
>   * adapt to the resolution.
> @@ -101,12 +330,25 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  {
>  	int ret = 0;
>  	resource_size_t base;
> +	struct ls2k_bmc_pdata *priv;
>  	struct simplefb_platform_data pd;
>  
>  	ret = pci_enable_device(dev);
>  	if (ret)
>  		return ret;
>  
> +	priv = devm_kzalloc(&dev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (IS_ERR(priv)) {
> +		ret = -ENOMEM;
> +		goto disable_pci;
> +	}
> +
> +	priv->dev = &dev->dev;
> +
> +	ret = ls2k_bmc_pdata_initial(dev, priv);
> +	if (ret)
> +		goto disable_pci;
> +
>  	ret = ls2k_bmc_get_video_mode(dev, &pd);
>  	if (ret)
>  		goto disable_pci;
> -- 
> 2.47.1
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH v2 1/3] mfd: ls2kbmc: Introduce Loongson-2K BMC MFD Core driver

[Binbin Zhou]

Hi Lee:

Thanks for your detailed review.

On Thu, May 22, 2025 at 5:22 PM Lee Jones <lee@kernel.org> wrote:
>
> Just "core driver" in the subject line, rather than "MFD core driver".
>
> > The Loongson-2K Board Management Controller provides an PCIe
> > interface to the host to access the feature implemented in the BMC.
> >
> > The BMC is assembled on a server similar to the server machine with
> > Loongson-3C6000 CPUs. It supports multiple sub-devices like DRM.
> >
> > Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> > Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > ---
> >  drivers/mfd/Kconfig       |  13 ++++
> >  drivers/mfd/Makefile      |   2 +
> >  drivers/mfd/ls2kbmc-mfd.c | 156 ++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 171 insertions(+)
> >  create mode 100644 drivers/mfd/ls2kbmc-mfd.c
> >
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 22b936310039..04e40085441d 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -2422,5 +2422,18 @@ config MFD_UPBOARD_FPGA
> >         To compile this driver as a module, choose M here: the module will be
> >         called upboard-fpga.
> >
> > +config MFD_LS2K_BMC
> > +     tristate "Loongson-2K Board Management Controller Support"
> > +     depends on LOONGARCH
> > +     default y if LOONGARCH
> > +     select MFD_CORE
> > +     help
> > +       Say yes here to add support for the Loongson-2K BMC
> > +       which is a Board Management Controller connected to the PCIe bus.
> > +       The device supports multiple sub-devices like DRM.
> > +       This driver provides common support for accessing the devices;
> > +       additional drivers must be enabled in order to use the
> > +       functionality of the BMC device.
>
> This paragraph has some odd line breaks.  Please re-align.
>
> >  endmenu
> >  endif
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index 948cbdf42a18..18960ea13b64 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -290,3 +290,5 @@ obj-$(CONFIG_MFD_RSMU_I2C)        += rsmu_i2c.o rsmu_core.o
> >  obj-$(CONFIG_MFD_RSMU_SPI)   += rsmu_spi.o rsmu_core.o
> >
> >  obj-$(CONFIG_MFD_UPBOARD_FPGA)       += upboard-fpga.o
> > +
> > +obj-$(CONFIG_MFD_LS2K_BMC)   += ls2kbmc-mfd.o
> > diff --git a/drivers/mfd/ls2kbmc-mfd.c b/drivers/mfd/ls2kbmc-mfd.c
> > new file mode 100644
> > index 000000000000..b309f6132c24
> > --- /dev/null
> > +++ b/drivers/mfd/ls2kbmc-mfd.c
> > @@ -0,0 +1,156 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Loongson-2K Board Management Controller (BMC) MFD Core Driver.
>
> Remove the MFD part.  It's not a thing - we made it up.
>
> > + * Copyright (C) 2024 Loongson Technology Corporation Limited.
>
> No changes since 2024?
>
> > + *
> > + * Originally written by Chong Qiao <qiaochong@loongson.cn>
> > + * Rewritten for mainline by Binbin Zhou <zhoubinbin@loongson.cn>
>
> "Mainline"
>
> Typically we just do:
>
> Authors:
>         Author One <one@corp.com>
>         Author Two <two@corp.com>
>
> > + */
> > +
> > +#include <linux/aperture.h>
> > +#include <linux/errno.h>
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/mfd/core.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/pci_ids.h>
> > +#include <linux/platform_data/simplefb.h>
> > +#include <linux/platform_device.h>
> > +
> > +#define LS2K_DISPLAY_RES_START (SZ_16M + SZ_2M)
> > +#define LS2K_IPMI_RES_SIZE   0x1c
> > +#define LS2K_IPMI0_RES_START (SZ_16M + 0xf00000)
> > +#define LS2K_IPMI1_RES_START (LS2K_IPMI0_RES_START + LS2K_IPMI_RES_SIZE)
> > +#define LS2K_IPMI2_RES_START (LS2K_IPMI1_RES_START + LS2K_IPMI_RES_SIZE)
> > +#define LS2K_IPMI3_RES_START (LS2K_IPMI2_RES_START + LS2K_IPMI_RES_SIZE)
> > +#define LS2K_IPMI4_RES_START (LS2K_IPMI3_RES_START + LS2K_IPMI_RES_SIZE)
>
> Line them _all_ up please.  One more tab should do it.
>
> > +static struct resource ls2k_display_resources[] = {
> > +     DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
> > +};
> > +
> > +static struct resource ls2k_ipmi0_resources[] = {
> > +     DEFINE_RES_MEM_NAMED(LS2K_IPMI0_RES_START, LS2K_IPMI_RES_SIZE, "ipmi0-res"),
> > +};
> > +
> > +static struct resource ls2k_ipmi1_resources[] = {
> > +     DEFINE_RES_MEM_NAMED(LS2K_IPMI1_RES_START, LS2K_IPMI_RES_SIZE, "ipmi1-res"),
> > +};
> > +
> > +static struct resource ls2k_ipmi2_resources[] = {
> > +     DEFINE_RES_MEM_NAMED(LS2K_IPMI2_RES_START, LS2K_IPMI_RES_SIZE, "ipmi2-res"),
> > +};
> > +
> > +static struct resource ls2k_ipmi3_resources[] = {
> > +     DEFINE_RES_MEM_NAMED(LS2K_IPMI3_RES_START, LS2K_IPMI_RES_SIZE, "ipmi3-res"),
> > +};
> > +
> > +static struct resource ls2k_ipmi4_resources[] = {
> > +     DEFINE_RES_MEM_NAMED(LS2K_IPMI4_RES_START, LS2K_IPMI_RES_SIZE, "ipmi4-res"),
> > +};
> > +
> > +static struct mfd_cell ls2k_bmc_cells[] = {
> > +     MFD_CELL_RES("simple-framebuffer", ls2k_display_resources),
> > +     MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi0_resources),
> > +     MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi1_resources),
> > +     MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi2_resources),
> > +     MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi3_resources),
> > +     MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
> > +};
> > +
> > +/*
> > + * Currently the Loongson-2K0500 BMC hardware does not have an i2c interface to
>
> I2C
>
> > + * adapt to the resolution.
>
> Remove the line break here.
>
> > + * We set the resolution by presetting "video=1280x1024-16@2M" to the bmc memory.
>
> BMC
>
> > + */
> > +static int ls2k_bmc_get_video_mode(struct pci_dev *pdev, struct simplefb_platform_data *pd)
> > +{
> > +     char *mode;
> > +     int depth, ret;
> > +
> > +     /* The pci mem bar last 16M is used to store the string. */
>
> PCI
>
> BAR's (maybe?)

/* The last 16M of PCI BAR0 is used to store the resolution string. */
>
> > +     mode = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0) + SZ_16M, SZ_16M);
> > +     if (!mode)
> > +             return -ENOMEM;
> > +
> > +     /* env at last 16M's beginning, first env is "video=" */
>
> This doesn't make sense to me - please reword.

How about:

/* The resolution field starts with the flag “video=”. */
>
> > +     if (!strncmp(mode, "video=", 6))
> > +             mode = mode + 6;
> > +
> > +     ret = kstrtoint(strsep(&mode, "x"), 10, &pd->width);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = kstrtoint(strsep(&mode, "-"), 10, &pd->height);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = kstrtoint(strsep(&mode, "@"), 10, &depth);
> > +     if (ret)
> > +             return ret;
> > +
> > +     pd->stride = pd->width * depth / 8;
> > +     pd->format = depth == 32 ? "a8r8g8b8" : "r5g6b5";
> > +
> > +     return 0;
> > +}
>
> Surely there is a standard format / API for this already?
>
>
Now, simplefb_platform_data is mainly described in DTS and parsed by simpledrm.
And when it is used as platform_data, there doesn't seem to be a
unified API to populate simplefb_platform_data. e.g. in
sysfb_simplefb[1], it is parsed using sysfb_parse_mode().

[1]: https://elixir.bootlin.com/linux/v6.15-rc1/source/drivers/firmware/sysfb_simplefb.c#L27
>
> > +static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
> > +{
> > +     int ret = 0;
>
> There is no need to pre-initialise this.
>
> > +     resource_size_t base;
> > +     struct simplefb_platform_data pd;
>
> Reverse these please (reverse Christmas tree is preferred).
> > +
> > +     ret = pci_enable_device(dev);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = ls2k_bmc_get_video_mode(dev, &pd);
> > +     if (ret)
> > +             goto disable_pci;
> > +
> > +     ls2k_bmc_cells[0].platform_data = &pd;
> > +     ls2k_bmc_cells[0].pdata_size = sizeof(pd);
> > +     base = dev->resource[0].start + LS2K_DISPLAY_RES_START;
> > +
> > +     /* Remove conflicting efifb device */
> > +     ret = aperture_remove_conflicting_devices(base, SZ_4M, "simple-framebuffer");
> > +     if (ret) {
> > +             dev_err(&dev->dev, "Remove firmware framebuffers failed: %d\n", ret);
>
> "Failed to removed firmware framebuffers"
>
> > +             goto disable_pci;
> > +     }
> > +
> > +     return devm_mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
> > +                                 ls2k_bmc_cells, ARRAY_SIZE(ls2k_bmc_cells),
> > +                                 &dev->resource[0], 0, NULL);
> > +
> > +disable_pci:
> > +     pci_disable_device(dev);
> > +     return ret;
> > +}
> > +
> > +static void ls2k_bmc_remove(struct pci_dev *dev)
> > +{
> > +     pci_disable_device(dev);
> > +}
> > +
> > +static struct pci_device_id ls2k_bmc_devices[] = {
> > +     { PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x1a05) },
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(pci, ls2k_bmc_devices);
> > +
> > +static struct pci_driver ls2k_bmc_driver = {
> > +     .name = "ls2k-bmc",
> > +     .id_table = ls2k_bmc_devices,
> > +     .probe = ls2k_bmc_probe,
> > +     .remove = ls2k_bmc_remove,
> > +};
> > +
>
> Remove this line.

All comments about code formatting will be changed in the next version.
>
> > +module_pci_driver(ls2k_bmc_driver);
> > +
> > +MODULE_DESCRIPTION("Loongson-2K BMC driver");
> > +MODULE_AUTHOR("Loongson Technology Corporation Limited");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.47.1
> >
>
> --
> Lee Jones [李琼斯]

-- 
Thanks.
Binbin

Re: [PATCH v2 0/3] LoongArch: Add Loongson-2K0500 BMC support

[Huacai Chen]

Hi, Binbin,

The title of this series can be unified from Loongson-2K500/Loongson-2K to LS2K.

Huacai

On Thu, May 15, 2025 at 10:32 AM Binbin Zhou <zhoubinbin@loongson.cn> wrote:
>
> Hi all:
>
> This patch set introduces the Loongson-2K0500 BMC.
>
> It is a PCIe device present on servers similar to the Loongson-3C6000.
> And it is a multifunctional device (MFD), such as display as a sub-function
> of it.
>
> 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: IPMI implementation
> patch-3: display, based on simpledrm
> patch-4: BMC reboot support
>
> Thanks.
>
> -------
> 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 MFD Core driver
>   ipmi: Add Loongson-2K BMC support
>   mfd: ls2kbmc: Add Loongson-2K BMC reset function 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 | 250 +++++++++++++++++++
>  drivers/mfd/Kconfig              |  13 +
>  drivers/mfd/Makefile             |   2 +
>  drivers/mfd/ls2kbmc-mfd.c        | 398 +++++++++++++++++++++++++++++++
>  7 files changed, 674 insertions(+)
>  create mode 100644 drivers/char/ipmi/ipmi_si_ls2k.c
>  create mode 100644 drivers/mfd/ls2kbmc-mfd.c
>
>
> base-commit: 9f2b0c15b752bb940e2eb6737bee30fff96d96b6
> --
> 2.47.1
>
>

Re: [PATCH v2 1/3] mfd: ls2kbmc: Introduce Loongson-2K BMC MFD Core driver

[Huacai Chen]

Hi, Binbin,

On Thu, May 15, 2025 at 10:32 AM Binbin Zhou <zhoubinbin@loongson.cn> wrote:
>
> The Loongson-2K Board Management Controller provides an PCIe
> interface to the host to access the feature implemented in the BMC.
>
> The BMC is assembled on a server similar to the server machine with
> Loongson-3C6000 CPUs. It supports multiple sub-devices like DRM.
>
> Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  drivers/mfd/Kconfig       |  13 ++++
>  drivers/mfd/Makefile      |   2 +
>  drivers/mfd/ls2kbmc-mfd.c | 156 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 171 insertions(+)
>  create mode 100644 drivers/mfd/ls2kbmc-mfd.c
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 22b936310039..04e40085441d 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2422,5 +2422,18 @@ config MFD_UPBOARD_FPGA
>           To compile this driver as a module, choose M here: the module will be
>           called upboard-fpga.
>
> +config MFD_LS2K_BMC
> +       tristate "Loongson-2K Board Management Controller Support"
> +       depends on LOONGARCH
> +       default y if LOONGARCH
> +       select MFD_CORE
> +       help
> +         Say yes here to add support for the Loongson-2K BMC
> +         which is a Board Management Controller connected to the PCIe bus.
> +         The device supports multiple sub-devices like DRM.
> +         This driver provides common support for accessing the devices;
> +         additional drivers must be enabled in order to use the
> +         functionality of the BMC device.
> +
>  endmenu
>  endif
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 948cbdf42a18..18960ea13b64 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -290,3 +290,5 @@ obj-$(CONFIG_MFD_RSMU_I2C)  += rsmu_i2c.o rsmu_core.o
>  obj-$(CONFIG_MFD_RSMU_SPI)     += rsmu_spi.o rsmu_core.o
>
>  obj-$(CONFIG_MFD_UPBOARD_FPGA) += upboard-fpga.o
> +
> +obj-$(CONFIG_MFD_LS2K_BMC)     += ls2kbmc-mfd.o
> diff --git a/drivers/mfd/ls2kbmc-mfd.c b/drivers/mfd/ls2kbmc-mfd.c
> new file mode 100644
> index 000000000000..b309f6132c24
> --- /dev/null
> +++ b/drivers/mfd/ls2kbmc-mfd.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Loongson-2K Board Management Controller (BMC) MFD Core Driver.
> + *
> + * Copyright (C) 2024 Loongson Technology Corporation Limited.
> + *
> + * Originally written by Chong Qiao <qiaochong@loongson.cn>
> + * Rewritten for mainline by Binbin Zhou <zhoubinbin@loongson.cn>
> + */
> +
> +#include <linux/aperture.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/pci_ids.h>
> +#include <linux/platform_data/simplefb.h>
> +#include <linux/platform_device.h>
> +
> +#define LS2K_DISPLAY_RES_START (SZ_16M + SZ_2M)
> +#define LS2K_IPMI_RES_SIZE     0x1c
> +#define LS2K_IPMI0_RES_START   (SZ_16M + 0xf00000)
> +#define LS2K_IPMI1_RES_START   (LS2K_IPMI0_RES_START + LS2K_IPMI_RES_SIZE)
> +#define LS2K_IPMI2_RES_START   (LS2K_IPMI1_RES_START + LS2K_IPMI_RES_SIZE)
> +#define LS2K_IPMI3_RES_START   (LS2K_IPMI2_RES_START + LS2K_IPMI_RES_SIZE)
> +#define LS2K_IPMI4_RES_START   (LS2K_IPMI3_RES_START + LS2K_IPMI_RES_SIZE)
> +
> +static struct resource ls2k_display_resources[] = {
> +       DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
> +};
> +
> +static struct resource ls2k_ipmi0_resources[] = {
> +       DEFINE_RES_MEM_NAMED(LS2K_IPMI0_RES_START, LS2K_IPMI_RES_SIZE, "ipmi0-res"),
> +};
> +
> +static struct resource ls2k_ipmi1_resources[] = {
> +       DEFINE_RES_MEM_NAMED(LS2K_IPMI1_RES_START, LS2K_IPMI_RES_SIZE, "ipmi1-res"),
> +};
> +
> +static struct resource ls2k_ipmi2_resources[] = {
> +       DEFINE_RES_MEM_NAMED(LS2K_IPMI2_RES_START, LS2K_IPMI_RES_SIZE, "ipmi2-res"),
> +};
> +
> +static struct resource ls2k_ipmi3_resources[] = {
> +       DEFINE_RES_MEM_NAMED(LS2K_IPMI3_RES_START, LS2K_IPMI_RES_SIZE, "ipmi3-res"),
> +};
> +
> +static struct resource ls2k_ipmi4_resources[] = {
> +       DEFINE_RES_MEM_NAMED(LS2K_IPMI4_RES_START, LS2K_IPMI_RES_SIZE, "ipmi4-res"),
> +};
> +
> +static struct mfd_cell ls2k_bmc_cells[] = {
> +       MFD_CELL_RES("simple-framebuffer", ls2k_display_resources),
> +       MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi0_resources),
> +       MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi1_resources),
> +       MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi2_resources),
> +       MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi3_resources),
> +       MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
> +};
> +
> +/*
> + * Currently the Loongson-2K0500 BMC hardware does not have an i2c interface to
Don't limit it to Loongson-2K0500, use Loongson-2K or LS2K instead.

Huacai

> + * adapt to the resolution.
> + * We set the resolution by presetting "video=1280x1024-16@2M" to the bmc memory.
> + */
> +static int ls2k_bmc_get_video_mode(struct pci_dev *pdev, struct simplefb_platform_data *pd)
> +{
> +       char *mode;
> +       int depth, ret;
> +
> +       /* The pci mem bar last 16M is used to store the string. */
> +       mode = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0) + SZ_16M, SZ_16M);
> +       if (!mode)
> +               return -ENOMEM;
> +
> +       /* env at last 16M's beginning, first env is "video=" */
> +       if (!strncmp(mode, "video=", 6))
> +               mode = mode + 6;
> +
> +       ret = kstrtoint(strsep(&mode, "x"), 10, &pd->width);
> +       if (ret)
> +               return ret;
> +
> +       ret = kstrtoint(strsep(&mode, "-"), 10, &pd->height);
> +       if (ret)
> +               return ret;
> +
> +       ret = kstrtoint(strsep(&mode, "@"), 10, &depth);
> +       if (ret)
> +               return ret;
> +
> +       pd->stride = pd->width * depth / 8;
> +       pd->format = depth == 32 ? "a8r8g8b8" : "r5g6b5";
> +
> +       return 0;
> +}
> +
> +static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
> +{
> +       int ret = 0;
> +       resource_size_t base;
> +       struct simplefb_platform_data pd;
> +
> +       ret = pci_enable_device(dev);
> +       if (ret)
> +               return ret;
> +
> +       ret = ls2k_bmc_get_video_mode(dev, &pd);
> +       if (ret)
> +               goto disable_pci;
> +
> +       ls2k_bmc_cells[0].platform_data = &pd;
> +       ls2k_bmc_cells[0].pdata_size = sizeof(pd);
> +       base = dev->resource[0].start + LS2K_DISPLAY_RES_START;
> +
> +       /* Remove conflicting efifb device */
> +       ret = aperture_remove_conflicting_devices(base, SZ_4M, "simple-framebuffer");
> +       if (ret) {
> +               dev_err(&dev->dev, "Remove firmware framebuffers failed: %d\n", ret);
> +               goto disable_pci;
> +       }
> +
> +       return devm_mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
> +                                   ls2k_bmc_cells, ARRAY_SIZE(ls2k_bmc_cells),
> +                                   &dev->resource[0], 0, NULL);
> +
> +disable_pci:
> +       pci_disable_device(dev);
> +       return ret;
> +}
> +
> +static void ls2k_bmc_remove(struct pci_dev *dev)
> +{
> +       pci_disable_device(dev);
> +}
> +
> +static struct pci_device_id ls2k_bmc_devices[] = {
> +       { PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x1a05) },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(pci, ls2k_bmc_devices);
> +
> +static struct pci_driver ls2k_bmc_driver = {
> +       .name = "ls2k-bmc",
> +       .id_table = ls2k_bmc_devices,
> +       .probe = ls2k_bmc_probe,
> +       .remove = ls2k_bmc_remove,
> +};
> +
> +module_pci_driver(ls2k_bmc_driver);
> +
> +MODULE_DESCRIPTION("Loongson-2K BMC driver");
> +MODULE_AUTHOR("Loongson Technology Corporation Limited");
> +MODULE_LICENSE("GPL");
> --
> 2.47.1
>
>

Re: [PATCH v2 3/3] mfd: ls2kbmc: Add Loongson-2K BMC reset function support

[Huacai Chen]

Hi, Binbin,

On Thu, May 15, 2025 at 10:32 AM Binbin Zhou <zhoubinbin@loongson.cn> wrote:
>
> Since the display is a sub-function of the Loongson-2K BMC, when the
> BMC reset, the entire BMC PCIe is disconnected, including the display
> which is interrupted.
>
> Not only do you need to save/restore the relevant PCIe registers
> throughout the reset process, but you also need to re-push the display
> to the monitor at the end.
>
> Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  drivers/mfd/ls2kbmc-mfd.c | 242 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 242 insertions(+)
>
> diff --git a/drivers/mfd/ls2kbmc-mfd.c b/drivers/mfd/ls2kbmc-mfd.c
> index b309f6132c24..4d35a13b3da5 100644
> --- a/drivers/mfd/ls2kbmc-mfd.c
> +++ b/drivers/mfd/ls2kbmc-mfd.c
> @@ -9,8 +9,11 @@
>   */
>
>  #include <linux/aperture.h>
> +#include <linux/delay.h>
>  #include <linux/errno.h>
>  #include <linux/init.h>
> +#include <linux/iopoll.h>
> +#include <linux/kbd_kern.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/core.h>
>  #include <linux/module.h>
> @@ -18,6 +21,8 @@
>  #include <linux/pci_ids.h>
>  #include <linux/platform_data/simplefb.h>
>  #include <linux/platform_device.h>
> +#include <linux/stop_machine.h>
> +#include <linux/vt_kern.h>
>
>  #define LS2K_DISPLAY_RES_START (SZ_16M + SZ_2M)
>  #define LS2K_IPMI_RES_SIZE     0x1c
> @@ -27,6 +32,9 @@
>  #define LS2K_IPMI3_RES_START   (LS2K_IPMI2_RES_START + LS2K_IPMI_RES_SIZE)
>  #define LS2K_IPMI4_RES_START   (LS2K_IPMI3_RES_START + LS2K_IPMI_RES_SIZE)
>
> +#define LS2K_BMC_RESET_DELAY   (60 * HZ)
> +#define LS2K_BMC_RESET_WAIT    (10 * HZ)
> +
>  static struct resource ls2k_display_resources[] = {
>         DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
>  };
> @@ -60,6 +68,227 @@ static struct mfd_cell ls2k_bmc_cells[] = {
>         MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
>  };
>
> +static const u32 index[] = { 0x4, 0x10, 0x14, 0x18, 0x1c, 0x20, 0x24,
> +                            0x30, 0x3c, 0x54, 0x58, 0x78, 0x7c, 0x80 };
> +static const u32 cindex[] = { 0x4, 0x10, 0x3c };
> +
> +struct ls2k_bmc_pci_data {
> +       u32 d80c;
> +       u32 d71c;
> +       u32 data[14];
> +       u32 cdata[3];
> +};
> +
> +struct ls2k_bmc_pdata {
> +       struct device *dev;
> +       struct work_struct reset_work;
> +       unsigned long reset_time;
> +       struct ls2k_bmc_pci_data pci_data;
> +};
> +
> +static bool ls2k_bmc_bar0_addr_is_set(struct pci_dev *ppdev)
> +{
> +       u32 addr;
> +
> +       pci_read_config_dword(ppdev, PCI_BASE_ADDRESS_0, &addr);
> +       addr &= PCI_BASE_ADDRESS_MEM_MASK;
> +
> +       return addr ? true : false;
> +}
> +
> +static bool ls2k_bmc_check_pcie_connected(struct pci_dev *parent,
> +                                         struct ls2k_bmc_pdata *priv)
> +{
> +       void __iomem *mmio;
> +       int sts, ret;
> +
> +       mmio = pci_iomap(parent, 0, 0x100);
> +       if (!mmio)
> +               return false;
> +
> +       writel(readl(mmio) | 0x8, mmio);
> +
> +       ret = readl_poll_timeout_atomic(mmio + 0xc, sts, (sts & 0x11) == 0x11,
> +                                       1000, 1000000);
> +       if (ret) {
> +               pci_iounmap(parent, mmio);
> +               dev_err(priv->dev, "PCIE train failed status=0x%x\n", sts);
> +               return false;
> +       }
> +
> +       pci_iounmap(parent, mmio);
> +       return true;
> +}
> +
> +static int ls2k_bmc_recover_pci_data(void *data)
> +{
> +       struct ls2k_bmc_pdata *priv = data;
> +       struct pci_dev *pdev = to_pci_dev(priv->dev);
> +       struct pci_dev *parent = pdev->bus->self;
> +       bool ready, dirty;
> +       u32 i;
> +
> +       pci_write_config_dword(parent, PCI_BASE_ADDRESS_2, 0);
> +       pci_write_config_dword(parent, PCI_BASE_ADDRESS_3, 0);
> +       pci_write_config_dword(parent, PCI_BASE_ADDRESS_4, 0);
> +
> +       for (i = 2000; i > 0 ; i--) {
> +               dirty = ls2k_bmc_bar0_addr_is_set(parent);
> +               if (!dirty)
> +                       break;
> +               mdelay(1);
> +       };
> +
> +       if (i == 0)
> +               dev_warn(priv->dev, "The PCI Bar is not cleared.\n");
> +
> +       for (i = 0; i < ARRAY_SIZE(index); i++)
> +               pci_write_config_dword(parent, index[i], priv->pci_data.data[i]);
> +
> +       pci_write_config_dword(parent, 0x80c, priv->pci_data.d80c);
> +       pci_write_config_dword(parent, 0x71c, priv->pci_data.d71c);
> +
> +       /* Check if the pcie is connected */
> +       ready = ls2k_bmc_check_pcie_connected(parent, priv);
> +       if (!ready)
> +               return ready;
> +
> +       dev_dbg(priv->dev, "The PCIE recovered done.\n");
> +
> +       /* Waiting for u-boot ddr config ready */
> +       mdelay(jiffies_to_msecs(LS2K_BMC_RESET_WAIT));
> +       ready = ls2k_bmc_bar0_addr_is_set(parent);
> +       if (!ready)
> +               return ready;
> +
> +       for (i = 0; i < ARRAY_SIZE(cindex); i++)
> +               pci_write_config_dword(pdev, cindex[i], priv->pci_data.cdata[i]);
> +
> +       return 0;
> +}
> +
> +static void ls2k_bmc_events_fn(struct work_struct *work)
> +{
> +       struct ls2k_bmc_pdata *priv = container_of(work, struct ls2k_bmc_pdata, reset_work);
> +
> +       /*
> +        * The pcie is lost when the BMC resets,
> +        * at which point access to the pcie from other CPUs
> +        * is suspended to prevent a crash.
> +        */
> +       stop_machine(ls2k_bmc_recover_pci_data, priv, NULL);
> +
> +#ifdef CONFIG_VT
> +       /* Re-push the display due to previous pcie loss. */
> +       set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> +#endif
> +
> +       dev_info(priv->dev, "Loongson-2K BMC recovered done.\n");
> +}
> +
> +static irqreturn_t ls2k_bmc_interrupt(int irq, void *arg)
> +{
> +       struct ls2k_bmc_pdata *priv = arg;
> +
> +       if (system_state != SYSTEM_RUNNING)
> +               return IRQ_HANDLED;
> +
> +       /* Skip interrupt in LS2K_BMC_RESET_DELAY */
> +       if (time_after(jiffies, priv->reset_time + LS2K_BMC_RESET_DELAY))
> +               schedule_work(&priv->reset_work);
> +
> +       priv->reset_time = jiffies;
> +
> +       return IRQ_HANDLED;
> +}
> +
> +#define BMC_RESET_GPIO                 14
> +#define LOONGSON_GPIO_REG_BASE         0x1fe00500
> +#define LOONGSON_GPIO_REG_SIZE         0x18
> +#define LOONGSON_GPIO_OEN              0x0
> +#define LOONGSON_GPIO_FUNC             0x4
> +#define LOONGSON_GPIO_INTPOL           0x10
> +#define LOONGSON_GPIO_INTEN            0x14
> +
> +/* The gpio interrupt is a watchdog interrupt that is triggered when the BMC resets. */
> +static int ls2k_bmc_gpio_reset_handler(struct ls2k_bmc_pdata *priv)
> +{
> +       int gsi = 16 + (BMC_RESET_GPIO & 7);
> +       void __iomem *gpio_base;
> +       int irq, ret = 0;
> +
> +       /* Since Loongson-3A hardware does not support GPIO interrupt cascade,
Loongson-3A here should be Loongson-3 or Loongson.

> +        * chip->gpio_to_irq() cannot be implemented,
> +        * here acpi_register_gsi() is used to get gpio irq.
> +        */
> +       irq = acpi_register_gsi(NULL, gsi, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW);
> +       if (irq < 0)
> +               return irq;
> +
> +       gpio_base = ioremap(LOONGSON_GPIO_REG_BASE, LOONGSON_GPIO_REG_SIZE);
> +       if (!gpio_base) {
> +               ret = PTR_ERR(gpio_base);
> +               goto acpi_failed;
> +       }
> +
> +       writel(readl(gpio_base + LOONGSON_GPIO_OEN) | BIT(BMC_RESET_GPIO),
> +              gpio_base + LOONGSON_GPIO_OEN);
> +       writel(readl(gpio_base + LOONGSON_GPIO_FUNC) & ~BIT(BMC_RESET_GPIO),
> +              gpio_base + LOONGSON_GPIO_FUNC);
> +       writel(readl(gpio_base + LOONGSON_GPIO_INTPOL) & ~BIT(BMC_RESET_GPIO),
> +              gpio_base + LOONGSON_GPIO_INTPOL);
> +       writel(readl(gpio_base + LOONGSON_GPIO_INTEN) | BIT(BMC_RESET_GPIO),
> +              gpio_base + LOONGSON_GPIO_INTEN);
> +
> +       ret = devm_request_irq(priv->dev, irq, ls2k_bmc_interrupt,
> +                              IRQF_SHARED | IRQF_TRIGGER_FALLING, "ls2kbmc gpio", priv);
> +       if (ret)
> +               dev_err(priv->dev, "ls2kbmc gpio request_irq(%d) failed\n", irq);
> +
> +       iounmap(gpio_base);
> +
> +acpi_failed:
> +       acpi_unregister_gsi(gsi);
> +
> +       return ret;
> +}
I prefer to inline the ls2k_bmc_gpio_reset_handler() function to
ls2k_bmc_pdata_initial(), or at least move it after
ls2k_bmc_save_pci_data().


Huacai

> +
> +static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> +{
> +       struct pci_dev *parent = pdev->bus->self;
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(index); i++)
> +               pci_read_config_dword(parent, index[i], &priv->pci_data.data[i]);
> +
> +       for (i = 0; i < ARRAY_SIZE(cindex); i++)
> +               pci_read_config_dword(pdev, cindex[i], &priv->pci_data.cdata[i]);
> +
> +       pci_read_config_dword(parent, 0x80c, &priv->pci_data.d80c);
> +       priv->pci_data.d80c = (priv->pci_data.d80c & ~(3 << 17)) | BIT(17);
> +
> +       pci_read_config_dword(parent, 0x71c, &priv->pci_data.d71c);
> +       priv->pci_data.d71c |= BIT(26);
> +}
> +
> +static int ls2k_bmc_pdata_initial(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> +{
> +       int ret;
> +
> +       ls2k_bmc_save_pci_data(pdev, priv);
> +
> +       INIT_WORK(&priv->reset_work, ls2k_bmc_events_fn);
> +
> +       ret = devm_request_irq(&pdev->dev, pdev->irq, ls2k_bmc_interrupt,
> +                              IRQF_SHARED | IRQF_TRIGGER_FALLING, "ls2kbmc pcie", priv);
> +       if (ret) {
> +               dev_err(priv->dev, "ls2kbmc pcie request_irq(%d) failed\n", pdev->irq);
> +               return ret;
> +       }
> +
> +       return ls2k_bmc_gpio_reset_handler(priv);
> +}
> +
>  /*
>   * Currently the Loongson-2K0500 BMC hardware does not have an i2c interface to
>   * adapt to the resolution.
> @@ -101,12 +330,25 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  {
>         int ret = 0;
>         resource_size_t base;
> +       struct ls2k_bmc_pdata *priv;
>         struct simplefb_platform_data pd;
>
>         ret = pci_enable_device(dev);
>         if (ret)
>                 return ret;
>
> +       priv = devm_kzalloc(&dev->dev, sizeof(*priv), GFP_KERNEL);
> +       if (IS_ERR(priv)) {
> +               ret = -ENOMEM;
> +               goto disable_pci;
> +       }
> +
> +       priv->dev = &dev->dev;
> +
> +       ret = ls2k_bmc_pdata_initial(dev, priv);
> +       if (ret)
> +               goto disable_pci;
> +
>         ret = ls2k_bmc_get_video_mode(dev, &pd);
>         if (ret)
>                 goto disable_pci;
> --
> 2.47.1
>

Re: [PATCH v2 3/3] mfd: ls2kbmc: Add Loongson-2K BMC reset function support

[Binbin Zhou]

Hi Lee:

Thanks for your detailed review.

On Thu, May 22, 2025 at 5:39 PM Lee Jones <lee@kernel.org> wrote:
>
> On Thu, 15 May 2025, Binbin Zhou wrote:
>
> > Since the display is a sub-function of the Loongson-2K BMC, when the
> > BMC reset, the entire BMC PCIe is disconnected, including the display
> > which is interrupted.
> >
> > Not only do you need to save/restore the relevant PCIe registers
> > throughout the reset process, but you also need to re-push the display
> > to the monitor at the end.
> >
> > Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> > Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > ---
> >  drivers/mfd/ls2kbmc-mfd.c | 242 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 242 insertions(+)
> >
> > diff --git a/drivers/mfd/ls2kbmc-mfd.c b/drivers/mfd/ls2kbmc-mfd.c
> > index b309f6132c24..4d35a13b3da5 100644
> > --- a/drivers/mfd/ls2kbmc-mfd.c
> > +++ b/drivers/mfd/ls2kbmc-mfd.c
> > @@ -9,8 +9,11 @@
> >   */
> >
> >  #include <linux/aperture.h>
> > +#include <linux/delay.h>
> >  #include <linux/errno.h>
> >  #include <linux/init.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/kbd_kern.h>
> >  #include <linux/kernel.h>
> >  #include <linux/mfd/core.h>
> >  #include <linux/module.h>
> > @@ -18,6 +21,8 @@
> >  #include <linux/pci_ids.h>
> >  #include <linux/platform_data/simplefb.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/stop_machine.h>
> > +#include <linux/vt_kern.h>
> >
> >  #define LS2K_DISPLAY_RES_START (SZ_16M + SZ_2M)
> >  #define LS2K_IPMI_RES_SIZE   0x1c
> > @@ -27,6 +32,9 @@
> >  #define LS2K_IPMI3_RES_START (LS2K_IPMI2_RES_START + LS2K_IPMI_RES_SIZE)
> >  #define LS2K_IPMI4_RES_START (LS2K_IPMI3_RES_START + LS2K_IPMI_RES_SIZE)
> >
> > +#define LS2K_BMC_RESET_DELAY (60 * HZ)
> > +#define LS2K_BMC_RESET_WAIT  (10 * HZ)
> > +
> >  static struct resource ls2k_display_resources[] = {
> >       DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
> >  };
> > @@ -60,6 +68,227 @@ static struct mfd_cell ls2k_bmc_cells[] = {
> >       MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
> >  };
> >
> > +static const u32 index[] = { 0x4, 0x10, 0x14, 0x18, 0x1c, 0x20, 0x24,
> > +                          0x30, 0x3c, 0x54, 0x58, 0x78, 0x7c, 0x80 };
> > +static const u32 cindex[] = { 0x4, 0x10, 0x3c };
>
> What are these?  You need to define them or at least provide a comment
> that describes what they are and how they're used.
>
> > +struct ls2k_bmc_pci_data {
> > +     u32 d80c;
> > +     u32 d71c;
> > +     u32 data[14];
> > +     u32 cdata[3];
>
> To the uninitiated, this is all gibberish.
>
> Either change the nomenclature or provide commentary.

These are the indexes of the PCI-E configuration space that need to be
saved, which can be found in include/uapi/linux/pci_regs.h. I will
redefine them as follows:

/* For LS2K BMC parent device -- LS7A2000 */
struct ls2k_bmc_parent_pci_data {
        u32 pci_command;
        u32 base_address[6];
        u32 rom_addreess;
        u32 interrupt_line;
        u32 msi_hi;
        u32 msi_lo;
        u32 devctl;
        u32 linkcap;
        u32 linkctl_sts;
        u32 symbol_timer;
        u32 gen2_ctrl;
};

/* For LS2K BMC */
struct ls2k_bmc_pci_data {
        u32 pci_command;
        u32 base_address0;
        u32 interrupt_line;
};

>
> > +};
> > +
> > +struct ls2k_bmc_pdata {
> > +     struct device *dev;
> > +     struct work_struct reset_work;
> > +     unsigned long reset_time;
>
> This always appears to be jiffies - is it used elsewhere?

Emm, It's only used in ls2k_bmc_interrupt(), and I didn't seem to need
to put it here.
>
> > +     struct ls2k_bmc_pci_data pci_data;
> > +};
> > +
> > +static bool ls2k_bmc_bar0_addr_is_set(struct pci_dev *ppdev)
> > +{
> > +     u32 addr;
> > +
> > +     pci_read_config_dword(ppdev, PCI_BASE_ADDRESS_0, &addr);
> > +     addr &= PCI_BASE_ADDRESS_MEM_MASK;
> > +
> > +     return addr ? true : false;
> > +}
> > +
> > +static bool ls2k_bmc_check_pcie_connected(struct pci_dev *parent,
> > +                                       struct ls2k_bmc_pdata *priv)
> > +{
> > +     void __iomem *mmio;
> > +     int sts, ret;
> > +
> > +     mmio = pci_iomap(parent, 0, 0x100);
>
> Why 0x100?  This should be defined.

This is ls7a2000 pcie port bar0 register, I will define it.
>
> > +     if (!mmio)
> > +             return false;
> > +
> > +     writel(readl(mmio) | 0x8, mmio);
> > +
> > +     ret = readl_poll_timeout_atomic(mmio + 0xc, sts, (sts & 0x11) == 0x11,
> > +                                     1000, 1000000);
> > +     if (ret) {
>
> All of the magic numbers in this function should be defined.

ok, I will do it.
>
> > +             pci_iounmap(parent, mmio);
> > +             dev_err(priv->dev, "PCIE train failed status=0x%x\n", sts);
> > +             return false;
> > +     }
> > +
> > +     pci_iounmap(parent, mmio);
> > +     return true;
> > +}
> > +
> > +static int ls2k_bmc_recover_pci_data(void *data)
> > +{
> > +     struct ls2k_bmc_pdata *priv = data;
> > +     struct pci_dev *pdev = to_pci_dev(priv->dev);
> > +     struct pci_dev *parent = pdev->bus->self;
> > +     bool ready, dirty;
> > +     u32 i;
> > +
> > +     pci_write_config_dword(parent, PCI_BASE_ADDRESS_2, 0);
> > +     pci_write_config_dword(parent, PCI_BASE_ADDRESS_3, 0);
> > +     pci_write_config_dword(parent, PCI_BASE_ADDRESS_4, 0);
>
> You need to provide commentary throughout this driver!
>
> > +
> > +     for (i = 2000; i > 0 ; i--) {
>
> Why 2000?  Please define and explain it.
>
> > +             dirty = ls2k_bmc_bar0_addr_is_set(parent);
>
> Since these are bools, you can put the function in an if() statement.

ok...
>
> > +             if (!dirty)
> > +                     break;
> > +             mdelay(1);
>
> Why?  What are we waiting for?

Since reading ls2k bmc will be stuck after the pcie port is reset, we
read bar0 of ls7a2000 (the parent device of ls2k bmc) to determine
whether it is reset or not, and we can only start the training after
the reset is successful.
>
> > +     };
> > +
> > +     if (i == 0)
> > +             dev_warn(priv->dev, "The PCI Bar is not cleared.\n");
>
> But it's okay to continue?  Why?
>
> > +     for (i = 0; i < ARRAY_SIZE(index); i++)
> > +             pci_write_config_dword(parent, index[i], priv->pci_data.data[i]);
> > +
> > +     pci_write_config_dword(parent, 0x80c, priv->pci_data.d80c);
> > +     pci_write_config_dword(parent, 0x71c, priv->pci_data.d71c);
> > +
> > +     /* Check if the pcie is connected */
>
> PCI-E
>
> > +     ready = ls2k_bmc_check_pcie_connected(parent, priv);
>
> As above.
>
> > +     if (!ready)
> > +             return ready;
> > +
> > +     dev_dbg(priv->dev, "The PCIE recovered done.\n");
>
> How useful is this once development is complete?
>
> > +     /* Waiting for u-boot ddr config ready */
>
> Will it always be U-Boot?
>
> Should be "U-Boot" and "DDR".
>
> > +     mdelay(jiffies_to_msecs(LS2K_BMC_RESET_WAIT));
>
> Why not define the value in ms already?
>
> > +     ready = ls2k_bmc_bar0_addr_is_set(parent);
>
> As above.
>
> > +     if (!ready)
> > +             return ready;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(cindex); i++)
> > +             pci_write_config_dword(pdev, cindex[i], priv->pci_data.cdata[i]);
> > +
> > +     return 0;
> > +}
> > +
> > +static void ls2k_bmc_events_fn(struct work_struct *work)
> > +{
> > +     struct ls2k_bmc_pdata *priv = container_of(work, struct ls2k_bmc_pdata, reset_work);
> > +
> > +     /*
> > +      * The pcie is lost when the BMC resets,
>
> Strange line-breaks.  Pick a limit (80-100, etc) and stick to it.
>
> > +      * at which point access to the pcie from other CPUs
> > +      * is suspended to prevent a crash.
> > +      */
> > +     stop_machine(ls2k_bmc_recover_pci_data, priv, NULL);
> > +
> > +#ifdef CONFIG_VT
>
> No avoidable #ifery in C-files please.

This is to avoid possible compilation failures caused by randconfig
(CONFIG_VT is not defined).
>
> > +     /* Re-push the display due to previous pcie loss. */
> > +     set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> > +#endif
> > +
> > +     dev_info(priv->dev, "Loongson-2K BMC recovered done.\n");
>
> Remove this line - we want clean bootlogs.
>
> > +}
> > +
> > +static irqreturn_t ls2k_bmc_interrupt(int irq, void *arg)
> > +{
> > +     struct ls2k_bmc_pdata *priv = arg;
> > +
> > +     if (system_state != SYSTEM_RUNNING)
> > +             return IRQ_HANDLED;
> > +
> > +     /* Skip interrupt in LS2K_BMC_RESET_DELAY */
> > +     if (time_after(jiffies, priv->reset_time + LS2K_BMC_RESET_DELAY))
> > +             schedule_work(&priv->reset_work);
> > +
> > +     priv->reset_time = jiffies;
> > +
> > +     return IRQ_HANDLED;
> > +}
> > +
> > +#define BMC_RESET_GPIO                       14
> > +#define LOONGSON_GPIO_REG_BASE               0x1fe00500
> > +#define LOONGSON_GPIO_REG_SIZE               0x18
> > +#define LOONGSON_GPIO_OEN            0x0
> > +#define LOONGSON_GPIO_FUNC           0x4
> > +#define LOONGSON_GPIO_INTPOL         0x10
> > +#define LOONGSON_GPIO_INTEN          0x14
>
> Put these up with the reset of the defines.
>
> > +/* The gpio interrupt is a watchdog interrupt that is triggered when the BMC resets. */
> > +static int ls2k_bmc_gpio_reset_handler(struct ls2k_bmc_pdata *priv)
>
> Why are we now doing GPIO things in here?  Why not a GPIO driver?

As I note below, our gpio controller hardware does not support
interrupt cascading for gpio_to_irq(), so this approach was taken.
>
> > +{
> > +     int gsi = 16 + (BMC_RESET_GPIO & 7);
> > +     void __iomem *gpio_base;
> > +     int irq, ret = 0;
> > +
> > +     /* Since Loongson-3A hardware does not support GPIO interrupt cascade,
> > +      * chip->gpio_to_irq() cannot be implemented,
> > +      * here acpi_register_gsi() is used to get gpio irq.
> > +      */
>
> Use proper multi-line commentary as per the Coding Style documentation.

I see.
>
> > +     irq = acpi_register_gsi(NULL, gsi, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW);
> > +     if (irq < 0)
> > +             return irq;
> > +
> > +     gpio_base = ioremap(LOONGSON_GPIO_REG_BASE, LOONGSON_GPIO_REG_SIZE);
> > +     if (!gpio_base) {
> > +             ret = PTR_ERR(gpio_base);
> > +             goto acpi_failed;
> > +     }
> > +
> > +     writel(readl(gpio_base + LOONGSON_GPIO_OEN) | BIT(BMC_RESET_GPIO),
> > +            gpio_base + LOONGSON_GPIO_OEN);
> > +     writel(readl(gpio_base + LOONGSON_GPIO_FUNC) & ~BIT(BMC_RESET_GPIO),
> > +            gpio_base + LOONGSON_GPIO_FUNC);
> > +     writel(readl(gpio_base + LOONGSON_GPIO_INTPOL) & ~BIT(BMC_RESET_GPIO),
> > +            gpio_base + LOONGSON_GPIO_INTPOL);
> > +     writel(readl(gpio_base + LOONGSON_GPIO_INTEN) | BIT(BMC_RESET_GPIO),
> > +            gpio_base + LOONGSON_GPIO_INTEN);
> > +
> > +     ret = devm_request_irq(priv->dev, irq, ls2k_bmc_interrupt,
> > +                            IRQF_SHARED | IRQF_TRIGGER_FALLING, "ls2kbmc gpio", priv);
> > +     if (ret)
> > +             dev_err(priv->dev, "ls2kbmc gpio request_irq(%d) failed\n", irq);
>
> "LS2KBMC" and "GPIO", etc.

All code formatting related issues will be fixed in the next release.
>
> > +
> > +     iounmap(gpio_base);
> > +
> > +acpi_failed:
> > +     acpi_unregister_gsi(gsi);
> > +
> > +     return ret;
> > +}
> > +
> > +static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > +{
> > +     struct pci_dev *parent = pdev->bus->self;
> > +     int i;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(index); i++)
> > +             pci_read_config_dword(parent, index[i], &priv->pci_data.data[i]);
> > +
> > +     for (i = 0; i < ARRAY_SIZE(cindex); i++)
> > +             pci_read_config_dword(pdev, cindex[i], &priv->pci_data.cdata[i]);
> > +
> > +     pci_read_config_dword(parent, 0x80c, &priv->pci_data.d80c);
> > +     priv->pci_data.d80c = (priv->pci_data.d80c & ~(3 << 17)) | BIT(17);
> > +
> > +     pci_read_config_dword(parent, 0x71c, &priv->pci_data.d71c);
> > +     priv->pci_data.d71c |= BIT(26);
>
> No magic numbers please.  Define them all.

I will define them as following:

#define LS2K_BMC_GEN2_CTL               0x80c
#define LS2K_BMC_SYMBOL_TIMER           0x71c

#define LS2K_BMC_GEN2_SPEED_CHANGE      BIT(17)
#define LS2K_BMC_CONF_PHY_TX            BIT(18)

#define LS2K_BMC_MASK_LEN_MATCH         BIT(26)

>
> > +}
> > +
> > +static int ls2k_bmc_pdata_initial(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > +{
> > +     int ret;
> > +
> > +     ls2k_bmc_save_pci_data(pdev, priv);
> > +
> > +     INIT_WORK(&priv->reset_work, ls2k_bmc_events_fn);
> > +
> > +     ret = devm_request_irq(&pdev->dev, pdev->irq, ls2k_bmc_interrupt,
> > +                            IRQF_SHARED | IRQF_TRIGGER_FALLING, "ls2kbmc pcie", priv);
> > +     if (ret) {
> > +             dev_err(priv->dev, "ls2kbmc pcie request_irq(%d) failed\n", pdev->irq);
> > +             return ret;
> > +     }
> > +
> > +     return ls2k_bmc_gpio_reset_handler(priv);
> > +}
> > +
> >  /*
> >   * Currently the Loongson-2K0500 BMC hardware does not have an i2c interface to
> >   * adapt to the resolution.
> > @@ -101,12 +330,25 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
> >  {
> >       int ret = 0;
> >       resource_size_t base;
> > +     struct ls2k_bmc_pdata *priv;
> >       struct simplefb_platform_data pd;
> >
> >       ret = pci_enable_device(dev);
> >       if (ret)
> >               return ret;
> >
> > +     priv = devm_kzalloc(&dev->dev, sizeof(*priv), GFP_KERNEL);
> > +     if (IS_ERR(priv)) {
> > +             ret = -ENOMEM;
> > +             goto disable_pci;
> > +     }
> > +
> > +     priv->dev = &dev->dev;
> > +
> > +     ret = ls2k_bmc_pdata_initial(dev, priv);
> > +     if (ret)
> > +             goto disable_pci;
> > +
> >       ret = ls2k_bmc_get_video_mode(dev, &pd);
> >       if (ret)
> >               goto disable_pci;
> > --
> > 2.47.1
> >
>
> --
> Lee Jones [李琼斯]

--
Thanks.
Binbin

Re: [PATCH v2 0/3] LoongArch: Add Loongson-2K0500 BMC support

[Huacai Chen]

Hi, Binbin,

I have some small comments, you can add "Reviewed-by: Huacai Chen
<chenhuacai@loongson.cn>" after you make changes.

Huacai

On Thu, May 15, 2025 at 10:32 AM Binbin Zhou <zhoubinbin@loongson.cn> wrote:
>
> Hi all:
>
> This patch set introduces the Loongson-2K0500 BMC.
>
> It is a PCIe device present on servers similar to the Loongson-3C6000.
> And it is a multifunctional device (MFD), such as display as a sub-function
> of it.
>
> 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: IPMI implementation
> patch-3: display, based on simpledrm
> patch-4: BMC reboot support
>
> Thanks.
>
> -------
> 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 MFD Core driver
>   ipmi: Add Loongson-2K BMC support
>   mfd: ls2kbmc: Add Loongson-2K BMC reset function 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 | 250 +++++++++++++++++++
>  drivers/mfd/Kconfig              |  13 +
>  drivers/mfd/Makefile             |   2 +
>  drivers/mfd/ls2kbmc-mfd.c        | 398 +++++++++++++++++++++++++++++++
>  7 files changed, 674 insertions(+)
>  create mode 100644 drivers/char/ipmi/ipmi_si_ls2k.c
>  create mode 100644 drivers/mfd/ls2kbmc-mfd.c
>
>
> base-commit: 9f2b0c15b752bb940e2eb6737bee30fff96d96b6
> --
> 2.47.1
>
>