[PATCH v7 0/3] LoongArch: Add Loongson-2K BMC support

[PATCH v7 0/3] LoongArch: Add Loongson-2K BMC support

[Binbin Zhou]

Hi all:

This patch set introduces the Loongson-2K BMC.

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

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 basic function support, such as display with simpledrm;
patch-2: BMC reset function support;
patch-3: IPMI implementation.

Thanks.

-------
V7:
Patch (1/3):
  - Fix build warning by lkp: Add depend on ACPI_GENERIC_GSI
    - https://lore.kernel.org/all/202507021011.sDAHGinj-lkp@intel.com/

Link to V6:
https://lore.kernel.org/all/cover.1750939357.git.zhoubinbin@loongson.cn/

V6:
- Add Acked-by tag from Corey, thanks;
Patch (1/3):
  - Fix build warning by lkp: Add depend on PCI
    - https://lore.kernel.org/all/202506210204.LVZc2VG2-lkp@intel.com/
    - https://lore.kernel.org/all/202506210231.ZWWNhofU-lkp@intel.com/
    - https://lore.kernel.org/all/202506210652.ipUFDU5B-lkp@intel.com/
    - https://lore.kernel.org/all/202506210343.XCHkzorp-lkp@intel.com/

Link to V5:
https://lore.kernel.org/all/cover.1750301674.git.zhoubinbin@loongson.cn/

V5:
Patch (1/3):
 - Rename ls2kbmc-mfd.c to ls2k-bmc-core.c;
 - Rename MFD_LS2K_BMC to MFD_LS2K_BMC_CORE and update its help text.
Patch (3/3):
 - Add an IPMI_LS2K config in the IPMI section that enables the IPMI
   interface and selects MFD_LS2K_BMC_CORE.

Link to V4:
https://lore.kernel.org/all/cover.1749731531.git.zhoubinbin@loongson.cn/

V4:
- Add Reviewed-by tag;
- Change the order of the patches.
Patch (1/3):
  - Fix build warning by lkp: Kconfig tristate -> bool
    - https://lore.kernel.org/all/202505312022.QmFmGE1F-lkp@intel.com/
 - Update commit message;
 - Move MFD_LS2K_BMC after MFD_INTEL_M10_BMC_PMCI in Kconfig and
   Makefile.
Patch (2/3):
  - Remove unnecessary newlines;
  - Rename ls2k_bmc_check_pcie_connected() to
    ls2k_bmc_pcie_is_connected();
  - Update comment message.
Patch (3/3):
  - Remove unnecessary newlines.

Link to V3:
https://lore.kernel.org/all/cover.1748505446.git.zhoubinbin@loongson.cn/

V3:
Patch (1/3):
 - Drop "MFD" in title and comment;
 - Fromatting code;
 - Add clearer comments.
Patch (2/3):
 - Rebase linux-ipmi/next tree;
 - Use readx()/writex() to read and write IPMI data instead of structure
   pointer references;
 - CONFIG_LOONGARCH -> MFD_LS2K_BMC;
 - Drop unused output.
Patch (3/3):
 - Inline the ls2k_bmc_gpio_reset_handler() function to ls2k_bmc_pdata_initial();
 - Add clearer comments.
 - Use proper multi-line commentary as per the Coding Style documentation;
 - Define all magic numbers.

Link to V2:
https://lore.kernel.org/all/cover.1747276047.git.zhoubinbin@loongson.cn/

V2:
- Drop ls2kdrm, use simpledrm instead.
Patch (1/3):
 - Use DEFINE_RES_MEM_NAMED/MFD_CELL_RES simplified code;
 - Add resolution fetching due to replacing the original display
   solution with simpledrm; 
 - Add aperture_remove_conflicting_devices() to avoid efifb
   conflict with simpledrm.
Patch (3/3):
 - This part of the function, moved from the original ls2kdrm to mfd;
 - Use set_console to implement the Re-push display function.

Link to V1:
https://lore.kernel.org/all/cover.1735550269.git.zhoubinbin@loongson.cn/

Binbin Zhou (3):
  mfd: ls2kbmc: Introduce Loongson-2K BMC core driver
  mfd: ls2kbmc: Add Loongson-2K BMC reset function support
  ipmi: Add Loongson-2K BMC support

 MAINTAINERS                      |   7 +
 drivers/char/ipmi/Kconfig        |   7 +
 drivers/char/ipmi/Makefile       |   1 +
 drivers/char/ipmi/ipmi_si.h      |   7 +
 drivers/char/ipmi/ipmi_si_intf.c |   4 +
 drivers/char/ipmi/ipmi_si_ls2k.c | 189 ++++++++++++
 drivers/mfd/Kconfig              |  13 +
 drivers/mfd/Makefile             |   2 +
 drivers/mfd/ls2k-bmc-core.c      | 484 +++++++++++++++++++++++++++++++
 9 files changed, 714 insertions(+)
 create mode 100644 drivers/char/ipmi/ipmi_si_ls2k.c
 create mode 100644 drivers/mfd/ls2k-bmc-core.c


base-commit: 2e8185c0f214d45cdbc20ee4e6796c561ba66c55
-- 
2.47.1

[PATCH v7 1/3] mfd: ls2kbmc: Introduce Loongson-2K BMC 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-3 CPU. It supports multiple sub-devices like DRM and IPMI.

Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
Acked-by: Corey Minyard <corey@minyard.net>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 MAINTAINERS                 |   6 ++
 drivers/mfd/Kconfig         |  13 +++
 drivers/mfd/Makefile        |   2 +
 drivers/mfd/ls2k-bmc-core.c | 156 ++++++++++++++++++++++++++++++++++++
 4 files changed, 177 insertions(+)
 create mode 100644 drivers/mfd/ls2k-bmc-core.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 0d053c45f7f9..4eb0f7b69d35 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14199,6 +14199,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
 F:	drivers/thermal/loongson2_thermal.c
 
+LOONGSON-2K Board Management Controller (BMC) DRIVER
+M:	Binbin Zhou <zhoubinbin@loongson.cn>
+M:	Chong Qiao <qiaochong@loongson.cn>
+S:	Maintained
+F:	drivers/mfd/ls2k-bmc-core.c
+
 LOONGSON EDAC DRIVER
 M:	Zhao Qunqin <zhaoqunqin@loongson.cn>
 L:	linux-edac@vger.kernel.org
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index c635790afa75..47cc8ea9d2ef 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2400,6 +2400,19 @@ config MFD_INTEL_M10_BMC_PMCI
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_LS2K_BMC_CORE
+	bool "Loongson-2K Board Management Controller Support"
+	depends on PCI && ACPI_GENERIC_GSI
+	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 display and IPMI. This driver provides common
+	  support for accessing the devices.
+
+	  The display is enabled by default in the driver, while the IPMI interface
+	  is enabled independently through the IPMI_LS2K option in the IPMI section.
+
 config MFD_QNAP_MCU
 	tristate "QNAP microcontroller unit core driver"
 	depends on SERIAL_DEV_BUS
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index ca351cb0ddcc..675b4ec6ef4c 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -284,6 +284,8 @@ obj-$(CONFIG_MFD_INTEL_M10_BMC_CORE)   += intel-m10-bmc-core.o
 obj-$(CONFIG_MFD_INTEL_M10_BMC_SPI)    += intel-m10-bmc-spi.o
 obj-$(CONFIG_MFD_INTEL_M10_BMC_PMCI)   += intel-m10-bmc-pmci.o
 
+obj-$(CONFIG_MFD_LS2K_BMC_CORE)		+= ls2k-bmc-core.o
+
 obj-$(CONFIG_MFD_ATC260X)	+= atc260x-core.o
 obj-$(CONFIG_MFD_ATC260X_I2C)	+= atc260x-i2c.o
 
diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
new file mode 100644
index 000000000000..50d560a4611c
--- /dev/null
+++ b/drivers/mfd/ls2k-bmc-core.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Loongson-2K Board Management Controller (BMC) Core Driver.
+ *
+ * Copyright (C) 2024-2025 Loongson Technology Corporation Limited.
+ *
+ * Authors:
+ *	Chong Qiao <qiaochong@loongson.cn>
+ *	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>
+
+/* LS2K BMC resources */
+#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-2K 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_parse_mode(struct pci_dev *pdev, struct simplefb_platform_data *pd)
+{
+	char *mode;
+	int depth, ret;
+
+	/* 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;
+
+	/* 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;
+}
+
+static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
+{
+	struct simplefb_platform_data pd;
+	resource_size_t base;
+	int ret;
+
+	ret = pci_enable_device(dev);
+	if (ret)
+		return ret;
+
+	ret = ls2k_bmc_parse_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, "Failed to removed firmware framebuffers: %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 Board Management Controller (BMC) Core driver");
+MODULE_AUTHOR("Loongson Technology Corporation Limited");
+MODULE_LICENSE("GPL");
-- 
2.47.1

[PATCH v7 2/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.

Quick overview of the entire LS2K BMC reset process:

There are two types of reset methods: soft reset (BMC-initiated reboot
of IPMI reset command) and BMC watchdog reset (watchdog timeout).

First, regardless of the method, an interrupt is generated (PCIe interrupt
for soft reset/GPIO interrupt for watchdog reset);

Second, during the interrupt process, the system enters bmc_reset_work,
clears the bus/IO/mem resources of the LS7A PCI-E bridge, waits for the BMC
reset to begin, then restores the parent device's PCI configuration space,
waits for the BMC reset to complete, and finally restores the BMC PCI
configuration space.

Display restoration occurs last.

Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
Acked-by: Corey Minyard <corey@minyard.net>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/mfd/ls2k-bmc-core.c | 328 ++++++++++++++++++++++++++++++++++++
 1 file changed, 328 insertions(+)

diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
index 50d560a4611c..1ae673f6a196 100644
--- a/drivers/mfd/ls2k-bmc-core.c
+++ b/drivers/mfd/ls2k-bmc-core.c
@@ -10,8 +10,12 @@
  */
 
 #include <linux/aperture.h>
+#include <linux/bitfield.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>
@@ -19,6 +23,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>
 
 /* LS2K BMC resources */
 #define LS2K_DISPLAY_RES_START		(SZ_16M + SZ_2M)
@@ -29,6 +35,48 @@
 #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 LS7A_PCI_CFG_SIZE		0x100
+
+/* LS7A bridge registers */
+#define LS7A_PCIE_PORT_CTL0		0x0
+#define LS7A_PCIE_PORT_STS1		0xC
+#define LS7A_GEN2_CTL			0x80C
+#define LS7A_SYMBOL_TIMER		0x71C
+
+/* Bits of LS7A_PCIE_PORT_CTL0 */
+#define LS2K_BMC_PCIE_LTSSM_ENABLE	BIT(3)
+
+/* Bits of LS7A_PCIE_PORT_STS1 */
+#define LS2K_BMC_PCIE_LTSSM_STS		GENMASK(5, 0)
+#define LS2K_BMC_PCIE_CONNECTED		0x11
+
+#define LS2K_BMC_PCIE_DELAY_US		1000
+#define LS2K_BMC_PCIE_TIMEOUT_US	1000000
+
+/* Bits of LS7A_GEN2_CTL */
+#define LS7A_GEN2_SPEED_CHANG		BIT(17)
+#define LS7A_CONF_PHY_TX		BIT(18)
+
+/* Bits of LS7A_SYMBOL_TIMER */
+#define LS7A_MASK_LEN_MATCH		BIT(26)
+
+/* Interval between interruptions */
+#define LS2K_BMC_INT_INTERVAL		(60 * HZ)
+
+/* Maximum time to wait for U-Boot and DDR to be ready with ms. */
+#define LS2K_BMC_RESET_WAIT_TIME	10000
+
+/* It's an experience value */
+#define LS7A_BAR0_CHECK_MAX_TIMES	2000
+
+#define LS2K_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
+
 static struct resource ls2k_display_resources[] = {
 	DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
 };
@@ -62,6 +110,273 @@ static struct mfd_cell ls2k_bmc_cells[] = {
 	MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
 };
 
+/* Index of the BMC PCI configuration space to be restored at BMC reset. */
+struct ls2k_bmc_pci_data {
+	u32 pci_command;
+	u32 base_address0;
+	u32 interrupt_line;
+};
+
+/* Index of the parent PCI configuration space to be restored at BMC reset. */
+struct ls2k_bmc_bridge_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;
+};
+
+struct ls2k_bmc_pdata {
+	struct device *dev;
+	struct work_struct bmc_reset_work;
+	struct ls2k_bmc_pci_data bmc_pci_data;
+	struct ls2k_bmc_bridge_pci_data bridge_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);
+
+	return addr & PCI_BASE_ADDRESS_MEM_MASK ? true : false;
+}
+
+static bool ls2k_bmc_pcie_is_connected(struct pci_dev *parent, struct ls2k_bmc_pdata *priv)
+{
+	void __iomem *base;
+	int sts, ret;
+
+	base = pci_iomap(parent, 0, LS7A_PCI_CFG_SIZE);
+	if (!base)
+		return false;
+
+	writel(readl(base + LS7A_PCIE_PORT_CTL0) | LS2K_BMC_PCIE_LTSSM_ENABLE,
+	       base + LS7A_PCIE_PORT_CTL0);
+
+	ret = readl_poll_timeout_atomic(base + LS7A_PCIE_PORT_STS1, sts,
+					(sts & LS2K_BMC_PCIE_LTSSM_STS) == LS2K_BMC_PCIE_CONNECTED,
+					LS2K_BMC_PCIE_DELAY_US, LS2K_BMC_PCIE_TIMEOUT_US);
+	if (ret) {
+		pci_iounmap(parent, base);
+		dev_err(priv->dev, "PCIE train failed status=0x%x\n", sts);
+		return false;
+	}
+
+	pci_iounmap(parent, base);
+	return true;
+}
+
+static void ls2k_bmc_restore_bridge_pci_data(struct pci_dev *parent, struct ls2k_bmc_pdata *priv)
+{
+	int base, i = 0;
+
+	pci_write_config_dword(parent, PCI_COMMAND, priv->bridge_pci_data.pci_command);
+
+	for (base = PCI_BASE_ADDRESS_0; base <= PCI_BASE_ADDRESS_5; base += 4, i++)
+		pci_write_config_dword(parent, base, priv->bridge_pci_data.base_address[i]);
+
+	pci_write_config_dword(parent, PCI_ROM_ADDRESS, priv->bridge_pci_data.rom_addreess);
+	pci_write_config_dword(parent, PCI_INTERRUPT_LINE, priv->bridge_pci_data.interrupt_line);
+
+	pci_write_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_LO,
+			       priv->bridge_pci_data.msi_lo);
+	pci_write_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_HI,
+			       priv->bridge_pci_data.msi_hi);
+	pci_write_config_dword(parent, parent->pcie_cap + PCI_EXP_DEVCTL,
+			       priv->bridge_pci_data.devctl);
+	pci_write_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCAP,
+			       priv->bridge_pci_data.linkcap);
+	pci_write_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCTL,
+			       priv->bridge_pci_data.linkctl_sts);
+
+	pci_write_config_dword(parent, LS7A_GEN2_CTL, priv->bridge_pci_data.gen2_ctrl);
+	pci_write_config_dword(parent, LS7A_SYMBOL_TIMER, priv->bridge_pci_data.symbol_timer);
+}
+
+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;
+	u32 i;
+
+	/*
+	 * Clear the bus, io and mem resources of the PCI-E bridge to zero, so that
+	 * the processor can not access the LS2K PCI-E port, to avoid crashing due to
+	 * the lack of return signal from accessing the LS2K PCI-E port.
+	 */
+	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);
+
+	/*
+	 * When the LS2K BMC is reset, the LS7A PCI-E port is also reset, and its PCI
+	 * BAR0 register is cleared. Due to the time gap between the GPIO interrupt
+	 * generation and the LS2K BMC reset, the LS7A PCI BAR0 register is read to
+	 * determine whether the reset has begun.
+	 */
+	for (i = LS7A_BAR0_CHECK_MAX_TIMES; i > 0 ; i--) {
+		if (!ls2k_bmc_bar0_addr_is_set(parent))
+			break;
+		mdelay(1);
+	};
+
+	if (i == 0)
+		return false;
+
+	ls2k_bmc_restore_bridge_pci_data(parent, priv);
+
+	/* Check if PCI-E is connected */
+	if (!ls2k_bmc_pcie_is_connected(parent, priv))
+		return false;
+
+	/* Waiting for U-Boot and DDR ready */
+	mdelay(LS2K_BMC_RESET_WAIT_TIME);
+	if (!ls2k_bmc_bar0_addr_is_set(parent))
+		return false;
+
+	/* Restore LS2K BMC PCI-E config data */
+	pci_write_config_dword(pdev, PCI_COMMAND, priv->bmc_pci_data.pci_command);
+	pci_write_config_dword(pdev, PCI_BASE_ADDRESS_0, priv->bmc_pci_data.base_address0);
+	pci_write_config_dword(pdev, PCI_INTERRUPT_LINE, priv->bmc_pci_data.interrupt_line);
+
+	return 0;
+}
+
+static void ls2k_bmc_events_fn(struct work_struct *work)
+{
+	struct ls2k_bmc_pdata *priv = container_of(work, struct ls2k_bmc_pdata, bmc_reset_work);
+
+	/*
+	 * The PCI-E is lost when the BMC resets, at which point access to the PCI-E
+	 * 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 PCI-E loss. */
+	set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
+#endif
+}
+
+static irqreturn_t ls2k_bmc_interrupt(int irq, void *arg)
+{
+	struct ls2k_bmc_pdata *priv = arg;
+	static unsigned long last_jiffies;
+
+	if (system_state != SYSTEM_RUNNING)
+		return IRQ_HANDLED;
+
+	/* Skip interrupt in LS2K_BMC_INT_INTERVAL */
+	if (time_after(jiffies, last_jiffies + LS2K_BMC_INT_INTERVAL)) {
+		schedule_work(&priv->bmc_reset_work);
+		last_jiffies = jiffies;
+	}
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Saves the BMC parent device (LS7A) and its own PCI configuration space registers
+ * that need to be restored after BMC reset.
+ */
+static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
+{
+	struct pci_dev *parent = pdev->bus->self;
+	int base, i = 0;
+
+	pci_read_config_dword(parent, PCI_COMMAND, &priv->bridge_pci_data.pci_command);
+
+	for (base = PCI_BASE_ADDRESS_0; base <= PCI_BASE_ADDRESS_5; base += 4, i++)
+		pci_read_config_dword(parent, base, &priv->bridge_pci_data.base_address[i]);
+
+	pci_read_config_dword(parent, PCI_ROM_ADDRESS, &priv->bridge_pci_data.rom_addreess);
+	pci_read_config_dword(parent, PCI_INTERRUPT_LINE, &priv->bridge_pci_data.interrupt_line);
+
+	pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_LO,
+			      &priv->bridge_pci_data.msi_lo);
+	pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_HI,
+			      &priv->bridge_pci_data.msi_hi);
+
+	pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_DEVCTL,
+			      &priv->bridge_pci_data.devctl);
+	pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCAP,
+			      &priv->bridge_pci_data.linkcap);
+	pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCTL,
+			      &priv->bridge_pci_data.linkctl_sts);
+
+	pci_read_config_dword(parent, LS7A_GEN2_CTL, &priv->bridge_pci_data.gen2_ctrl);
+	priv->bridge_pci_data.gen2_ctrl |= FIELD_PREP(LS7A_GEN2_SPEED_CHANG, 0x1)
+					| FIELD_PREP(LS7A_CONF_PHY_TX, 0x0);
+
+	pci_read_config_dword(parent, LS7A_SYMBOL_TIMER, &priv->bridge_pci_data.symbol_timer);
+	priv->bridge_pci_data.symbol_timer |= LS7A_MASK_LEN_MATCH;
+
+	pci_read_config_dword(pdev, PCI_COMMAND, &priv->bmc_pci_data.pci_command);
+	pci_read_config_dword(pdev, PCI_BASE_ADDRESS_0, &priv->bmc_pci_data.base_address0);
+	pci_read_config_dword(pdev, PCI_INTERRUPT_LINE, &priv->bmc_pci_data.interrupt_line);
+}
+
+static int ls2k_bmc_pdata_initial(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
+{
+	int gsi = 16 + (LS2K_BMC_RESET_GPIO & 7);
+	void __iomem *gpio_base;
+	int irq, ret;
+
+	ls2k_bmc_save_pci_data(pdev, priv);
+
+	INIT_WORK(&priv->bmc_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 PCI-E request_irq(%d) failed\n", pdev->irq);
+		return ret;
+	}
+
+	/*
+	 * Since gpio_chip->to_irq is not implemented in the Loongson-3 GPIO driver,
+	 * acpi_register_gsi() is used to obtain the GPIO irq. The GPIO interrupt is a
+	 * watchdog interrupt that is triggered when the BMC resets.
+	 */
+	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(LS2K_BMC_RESET_GPIO),
+	       gpio_base + LOONGSON_GPIO_OEN);
+	writel(readl(gpio_base + LOONGSON_GPIO_FUNC) & ~BIT(LS2K_BMC_RESET_GPIO),
+	       gpio_base + LOONGSON_GPIO_FUNC);
+	writel(readl(gpio_base + LOONGSON_GPIO_INTPOL) & ~BIT(LS2K_BMC_RESET_GPIO),
+	       gpio_base + LOONGSON_GPIO_INTPOL);
+	writel(readl(gpio_base + LOONGSON_GPIO_INTEN) | BIT(LS2K_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;
+}
+
 /*
  * Currently the Loongson-2K 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.
@@ -101,6 +416,7 @@ static int ls2k_bmc_parse_mode(struct pci_dev *pdev, struct simplefb_platform_da
 static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	struct simplefb_platform_data pd;
+	struct ls2k_bmc_pdata *priv;
 	resource_size_t base;
 	int ret;
 
@@ -108,6 +424,18 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	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_parse_mode(dev, &pd);
 	if (ret)
 		goto disable_pci;
-- 
2.47.1

[PATCH v7 3/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, fifo flag
is used to ensure data consistency.

The single KCS message block is as follows:

+-------------------------------------------------------------------------+
|FIFO flags| KCS register data | CMD data | KCS version | WR REQ | WR ACK |
+-------------------------------------------------------------------------+

Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
Acked-by: Corey Minyard <corey@minyard.net>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 MAINTAINERS                      |   1 +
 drivers/char/ipmi/Kconfig        |   7 ++
 drivers/char/ipmi/Makefile       |   1 +
 drivers/char/ipmi/ipmi_si.h      |   7 ++
 drivers/char/ipmi/ipmi_si_intf.c |   4 +
 drivers/char/ipmi/ipmi_si_ls2k.c | 189 +++++++++++++++++++++++++++++++
 6 files changed, 209 insertions(+)
 create mode 100644 drivers/char/ipmi/ipmi_si_ls2k.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4eb0f7b69d35..a585a35aaa73 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14203,6 +14203,7 @@ LOONGSON-2K Board Management Controller (BMC) DRIVER
 M:	Binbin Zhou <zhoubinbin@loongson.cn>
 M:	Chong Qiao <qiaochong@loongson.cn>
 S:	Maintained
+F:	drivers/char/ipmi/ipmi_si_ls2k.c
 F:	drivers/mfd/ls2k-bmc-core.c
 
 LOONGSON EDAC DRIVER
diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
index f4adc6feb3b2..92bed266d07c 100644
--- a/drivers/char/ipmi/Kconfig
+++ b/drivers/char/ipmi/Kconfig
@@ -84,6 +84,13 @@ config IPMI_IPMB
 	  bus, and it also supports direct messaging on the bus using
 	  IPMB direct messages.  This module requires I2C support.
 
+config IPMI_LS2K
+	bool 'Loongson-2K IPMI interface'
+	depends on LOONGARCH
+	select MFD_LS2K_BMC_CORE
+	help
+	  Provides a driver for Loongson-2K IPMI interfaces.
+
 config IPMI_POWERNV
 	depends on PPC_POWERNV
 	tristate 'POWERNV (OPAL firmware) IPMI interface'
diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
index e0944547c9d0..4ea450a82242 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_IPMI_LS2K) += 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 508c3fd45877..687835b53da5 100644
--- a/drivers/char/ipmi/ipmi_si.h
+++ b/drivers/char/ipmi/ipmi_si.h
@@ -101,6 +101,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_IPMI_LS2K
+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 bb42dfe1c6a8..9c38aca16fd0 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -2121,6 +2121,8 @@ static int __init init_ipmi_si(void)
 
 	ipmi_si_pci_init();
 
+	ipmi_si_ls2k_init();
+
 	ipmi_si_parisc_init();
 
 	mutex_lock(&smi_infos_lock);
@@ -2335,6 +2337,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..45442c257efd
--- /dev/null
+++ b/drivers/char/ipmi/ipmi_si_ls2k.c
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Loongson-2K BMC IPMI interface
+ *
+ * Copyright (C) 2024-2025 Loongson Technology Corporation Limited.
+ *
+ * Authors:
+ *	Chong Qiao <qiaochong@loongson.cn>
+ *	Binbin Zhou <zhoubinbin@loongson.cn>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/types.h>
+
+#include "ipmi_si.h"
+
+#define LS2K_KCS_FIFO_IBFH	0x0
+#define LS2K_KCS_FIFO_IBFT	0x1
+#define LS2K_KCS_FIFO_OBFH	0x2
+#define LS2K_KCS_FIFO_OBFT	0x3
+
+/* KCS registers */
+#define LS2K_KCS_REG_STS	0x4
+#define LS2K_KCS_REG_DATA_OUT	0x5
+#define LS2K_KCS_REG_DATA_IN	0x6
+#define LS2K_KCS_REG_CMD	0x8
+
+#define LS2K_KCS_CMD_DATA	0xa
+#define LS2K_KCS_VERSION	0xb
+#define LS2K_KCS_WR_REQ		0xc
+#define LS2K_KCS_WR_ACK		0x10
+
+#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)
+
+static bool ls2k_registered;
+
+static unsigned char ls2k_mem_inb_v0(const struct si_sm_io *io, unsigned int offset)
+{
+	void __iomem *addr = io->addr;
+	int reg_offset;
+
+	if (offset & BIT(0)) {
+		reg_offset = LS2K_KCS_REG_STS;
+	} else {
+		writeb(readb(addr + LS2K_KCS_REG_STS) & ~LS2K_KCS_STS_OBF, addr + LS2K_KCS_REG_STS);
+		reg_offset = LS2K_KCS_REG_DATA_OUT;
+	}
+
+	return readb(addr + reg_offset);
+}
+
+static unsigned char ls2k_mem_inb_v1(const struct si_sm_io *io, unsigned int offset)
+{
+	void __iomem *addr = io->addr;
+	unsigned char inb = 0, cmd;
+	bool obf, ibf;
+
+	obf = readb(addr + LS2K_KCS_FIFO_OBFH) ^ readb(addr + LS2K_KCS_FIFO_OBFT);
+	ibf = readb(addr + LS2K_KCS_FIFO_IBFH) ^ readb(addr + LS2K_KCS_FIFO_IBFT);
+	cmd = readb(addr + LS2K_KCS_CMD_DATA);
+
+	if (offset & BIT(0)) {
+		inb = readb(addr + LS2K_KCS_REG_STS) & ~LS2K_KCS_DATA_MASK;
+		inb |= FIELD_PREP(LS2K_KCS_STS_OBF, obf)
+		    | FIELD_PREP(LS2K_KCS_STS_IBF, ibf)
+		    | FIELD_PREP(LS2K_KCS_STS_CMD, cmd);
+	} else {
+		inb = readb(addr + LS2K_KCS_REG_DATA_OUT);
+		writeb(readb(addr + LS2K_KCS_FIFO_OBFH), addr + LS2K_KCS_FIFO_OBFT);
+	}
+
+	return inb;
+}
+
+static void ls2k_mem_outb_v0(const struct si_sm_io *io, unsigned int offset,
+			     unsigned char val)
+{
+	void __iomem *addr = io->addr;
+	unsigned char sts = readb(addr + LS2K_KCS_REG_STS);
+	int reg_offset;
+
+	if (sts & LS2K_KCS_STS_IBF)
+		return;
+
+	if (offset & BIT(0)) {
+		reg_offset = LS2K_KCS_REG_CMD;
+		sts |= LS2K_KCS_STS_CMD;
+	} else {
+		reg_offset = LS2K_KCS_REG_DATA_IN;
+		sts &= ~LS2K_KCS_STS_CMD;
+	}
+
+	writew(val, addr + reg_offset);
+	writeb(sts | LS2K_KCS_STS_IBF, addr + LS2K_KCS_REG_STS);
+	writel(readl(addr + LS2K_KCS_WR_REQ) + 1, addr + LS2K_KCS_WR_REQ);
+}
+
+static void ls2k_mem_outb_v1(const struct si_sm_io *io, unsigned int offset,
+			     unsigned char val)
+{
+	void __iomem *addr = io->addr;
+	unsigned char ibfh, ibft;
+	int reg_offset;
+
+	ibfh = readb(addr + LS2K_KCS_FIFO_IBFH);
+	ibft = readb(addr + LS2K_KCS_FIFO_IBFT);
+
+	if (ibfh ^ ibft)
+		return;
+
+	reg_offset = (offset & BIT(0)) ? LS2K_KCS_REG_CMD : LS2K_KCS_REG_DATA_IN;
+	writew(val, addr + reg_offset);
+
+	writeb(offset & BIT(0), addr + LS2K_KCS_CMD_DATA);
+	writeb(!ibft, addr + LS2K_KCS_FIFO_IBFH);
+	writel(readl(addr + LS2K_KCS_WR_REQ) + 1, addr + LS2K_KCS_WR_REQ);
+}
+
+static void ls2k_mem_cleanup(struct si_sm_io *io)
+{
+	if (io->addr)
+		iounmap(io->addr);
+}
+
+static int ipmi_ls2k_mem_setup(struct si_sm_io *io)
+{
+	unsigned char version;
+
+	io->addr = ioremap(io->addr_data, io->regspacing);
+	if (!io->addr)
+		return -EIO;
+
+	version = readb(io->addr + LS2K_KCS_VERSION);
+
+	io->inputb = version ? ls2k_mem_inb_v1 : ls2k_mem_inb_v0;
+	io->outputb = version ? ls2k_mem_outb_v1 : ls2k_mem_outb_v0;
+	io->io_cleanup = ls2k_mem_cleanup;
+
+	return 0;
+}
+
+static int ipmi_ls2k_probe(struct platform_device *pdev)
+{
+	struct si_sm_io io;
+
+	memset(&io, 0, sizeof(io));
+
+	io.si_info	= &ipmi_kcs_si_info;
+	io.io_setup	= ipmi_ls2k_mem_setup;
+	io.addr_data	= pdev->resource[0].start;
+	io.regspacing	= resource_size(&pdev->resource[0]);
+	io.dev		= &pdev->dev;
+
+	dev_dbg(&pdev->dev, "addr 0x%lx, spacing %d.\n", io.addr_data, io.regspacing);
+
+	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,
+};
+
+void ipmi_si_ls2k_init(void)
+{
+	platform_driver_register(&ipmi_ls2k_platform_driver);
+	ls2k_registered = true;
+}
+
+void ipmi_si_ls2k_shutdown(void)
+{
+	if (ls2k_registered)
+		platform_driver_unregister(&ipmi_ls2k_platform_driver);
+}
-- 
2.47.1

Re: [PATCH v7 1/3] mfd: ls2kbmc: Introduce Loongson-2K BMC core driver

[Lee Jones]

On Fri, 04 Jul 2025, Binbin Zhou 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-3 CPU. It supports multiple sub-devices like DRM and IPMI.
> 
> Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> Acked-by: Corey Minyard <corey@minyard.net>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  MAINTAINERS                 |   6 ++
>  drivers/mfd/Kconfig         |  13 +++
>  drivers/mfd/Makefile        |   2 +
>  drivers/mfd/ls2k-bmc-core.c | 156 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 177 insertions(+)
>  create mode 100644 drivers/mfd/ls2k-bmc-core.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0d053c45f7f9..4eb0f7b69d35 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14199,6 +14199,12 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
>  F:	drivers/thermal/loongson2_thermal.c
>  
> +LOONGSON-2K Board Management Controller (BMC) DRIVER
> +M:	Binbin Zhou <zhoubinbin@loongson.cn>
> +M:	Chong Qiao <qiaochong@loongson.cn>
> +S:	Maintained
> +F:	drivers/mfd/ls2k-bmc-core.c
> +
>  LOONGSON EDAC DRIVER
>  M:	Zhao Qunqin <zhaoqunqin@loongson.cn>
>  L:	linux-edac@vger.kernel.org
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index c635790afa75..47cc8ea9d2ef 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2400,6 +2400,19 @@ config MFD_INTEL_M10_BMC_PMCI
>  	  additional drivers must be enabled in order to use the functionality
>  	  of the device.
>  
> +config MFD_LS2K_BMC_CORE
> +	bool "Loongson-2K Board Management Controller Support"
> +	depends on PCI && ACPI_GENERIC_GSI
> +	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 display and IPMI. This driver provides common
> +	  support for accessing the devices.
> +
> +	  The display is enabled by default in the driver, while the IPMI interface
> +	  is enabled independently through the IPMI_LS2K option in the IPMI section.
> +
>  config MFD_QNAP_MCU
>  	tristate "QNAP microcontroller unit core driver"
>  	depends on SERIAL_DEV_BUS
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index ca351cb0ddcc..675b4ec6ef4c 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -284,6 +284,8 @@ obj-$(CONFIG_MFD_INTEL_M10_BMC_CORE)   += intel-m10-bmc-core.o
>  obj-$(CONFIG_MFD_INTEL_M10_BMC_SPI)    += intel-m10-bmc-spi.o
>  obj-$(CONFIG_MFD_INTEL_M10_BMC_PMCI)   += intel-m10-bmc-pmci.o
>  
> +obj-$(CONFIG_MFD_LS2K_BMC_CORE)		+= ls2k-bmc-core.o
> +
>  obj-$(CONFIG_MFD_ATC260X)	+= atc260x-core.o
>  obj-$(CONFIG_MFD_ATC260X_I2C)	+= atc260x-i2c.o
>  
> diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> new file mode 100644
> index 000000000000..50d560a4611c
> --- /dev/null
> +++ b/drivers/mfd/ls2k-bmc-core.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Loongson-2K Board Management Controller (BMC) Core Driver.
> + *
> + * Copyright (C) 2024-2025 Loongson Technology Corporation Limited.
> + *
> + * Authors:
> + *	Chong Qiao <qiaochong@loongson.cn>
> + *	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>
> +
> +/* LS2K BMC resources */
> +#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-2K 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_parse_mode(struct pci_dev *pdev, struct simplefb_platform_data *pd)
> +{
> +	char *mode;
> +	int depth, ret;
> +
> +	/* 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;
> +
> +	/* 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;
> +}
> +
> +static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
> +{
> +	struct simplefb_platform_data pd;
> +	resource_size_t base;
> +	int ret;
> +
> +	ret = pci_enable_device(dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = ls2k_bmc_parse_mode(dev, &pd);
> +	if (ret)
> +		goto disable_pci;
> +
> +	ls2k_bmc_cells[0].platform_data = &pd;
> +	ls2k_bmc_cells[0].pdata_size = sizeof(pd);

This is fragile.

Please identify the elements in ls2k_bmc_cells and use it to index here.

See: `static struct mfd_cell as3711_subdevs`

> +	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, "Failed to removed firmware framebuffers: %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 Board Management Controller (BMC) Core driver");
> +MODULE_AUTHOR("Loongson Technology Corporation Limited");
> +MODULE_LICENSE("GPL");
> -- 
> 2.47.1
> 

-- 
Lee Jones [李琼斯]

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

[Lee Jones]

On Fri, 04 Jul 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.
> 
> Quick overview of the entire LS2K BMC reset process:
> 
> There are two types of reset methods: soft reset (BMC-initiated reboot
> of IPMI reset command) and BMC watchdog reset (watchdog timeout).
> 
> First, regardless of the method, an interrupt is generated (PCIe interrupt
> for soft reset/GPIO interrupt for watchdog reset);
> 
> Second, during the interrupt process, the system enters bmc_reset_work,
> clears the bus/IO/mem resources of the LS7A PCI-E bridge, waits for the BMC
> reset to begin, then restores the parent device's PCI configuration space,
> waits for the BMC reset to complete, and finally restores the BMC PCI
> configuration space.
> 
> Display restoration occurs last.
> 
> Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> Acked-by: Corey Minyard <corey@minyard.net>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  drivers/mfd/ls2k-bmc-core.c | 328 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 328 insertions(+)
> 
> diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> index 50d560a4611c..1ae673f6a196 100644
> --- a/drivers/mfd/ls2k-bmc-core.c
> +++ b/drivers/mfd/ls2k-bmc-core.c
> @@ -10,8 +10,12 @@
>   */
>  
>  #include <linux/aperture.h>
> +#include <linux/bitfield.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>
> @@ -19,6 +23,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>
>  
>  /* LS2K BMC resources */
>  #define LS2K_DISPLAY_RES_START		(SZ_16M + SZ_2M)
> @@ -29,6 +35,48 @@
>  #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 LS7A_PCI_CFG_SIZE		0x100
> +
> +/* LS7A bridge registers */
> +#define LS7A_PCIE_PORT_CTL0		0x0
> +#define LS7A_PCIE_PORT_STS1		0xC
> +#define LS7A_GEN2_CTL			0x80C
> +#define LS7A_SYMBOL_TIMER		0x71C
> +
> +/* Bits of LS7A_PCIE_PORT_CTL0 */
> +#define LS2K_BMC_PCIE_LTSSM_ENABLE	BIT(3)
> +
> +/* Bits of LS7A_PCIE_PORT_STS1 */
> +#define LS2K_BMC_PCIE_LTSSM_STS		GENMASK(5, 0)
> +#define LS2K_BMC_PCIE_CONNECTED		0x11
> +
> +#define LS2K_BMC_PCIE_DELAY_US		1000
> +#define LS2K_BMC_PCIE_TIMEOUT_US	1000000
> +
> +/* Bits of LS7A_GEN2_CTL */
> +#define LS7A_GEN2_SPEED_CHANG		BIT(17)
> +#define LS7A_CONF_PHY_TX		BIT(18)
> +
> +/* Bits of LS7A_SYMBOL_TIMER */
> +#define LS7A_MASK_LEN_MATCH		BIT(26)
> +
> +/* Interval between interruptions */
> +#define LS2K_BMC_INT_INTERVAL		(60 * HZ)
> +
> +/* Maximum time to wait for U-Boot and DDR to be ready with ms. */
> +#define LS2K_BMC_RESET_WAIT_TIME	10000
> +
> +/* It's an experience value */
> +#define LS7A_BAR0_CHECK_MAX_TIMES	2000
> +
> +#define LS2K_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
> +
>  static struct resource ls2k_display_resources[] = {
>  	DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
>  };
> @@ -62,6 +110,273 @@ static struct mfd_cell ls2k_bmc_cells[] = {
>  	MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
>  };
>  
> +/* Index of the BMC PCI configuration space to be restored at BMC reset. */
> +struct ls2k_bmc_pci_data {
> +	u32 pci_command;
> +	u32 base_address0;
> +	u32 interrupt_line;
> +};
> +
> +/* Index of the parent PCI configuration space to be restored at BMC reset. */
> +struct ls2k_bmc_bridge_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;
> +};
> +
> +struct ls2k_bmc_pdata {
> +	struct device *dev;
> +	struct work_struct bmc_reset_work;
> +	struct ls2k_bmc_pci_data bmc_pci_data;
> +	struct ls2k_bmc_bridge_pci_data bridge_pci_data;
> +};
> +
> +static bool ls2k_bmc_bar0_addr_is_set(struct pci_dev *ppdev)

Nit: This is usually called pdev.

> +{
> +	u32 addr;
> +
> +	pci_read_config_dword(ppdev, PCI_BASE_ADDRESS_0, &addr);
> +
> +	return addr & PCI_BASE_ADDRESS_MEM_MASK ? true : false;
> +}
> +
> +static bool ls2k_bmc_pcie_is_connected(struct pci_dev *parent, struct ls2k_bmc_pdata *priv)

Nit: Rename priv to ddata.

> +{
> +	void __iomem *base;
> +	int sts, ret;
> +
> +	base = pci_iomap(parent, 0, LS7A_PCI_CFG_SIZE);
> +	if (!base)
> +		return false;
> +
> +	writel(readl(base + LS7A_PCIE_PORT_CTL0) | LS2K_BMC_PCIE_LTSSM_ENABLE,
> +	       base + LS7A_PCIE_PORT_CTL0);
> +
> +	ret = readl_poll_timeout_atomic(base + LS7A_PCIE_PORT_STS1, sts,
> +					(sts & LS2K_BMC_PCIE_LTSSM_STS) == LS2K_BMC_PCIE_CONNECTED,
> +					LS2K_BMC_PCIE_DELAY_US, LS2K_BMC_PCIE_TIMEOUT_US);
> +	if (ret) {
> +		pci_iounmap(parent, base);
> +		dev_err(priv->dev, "PCIE train failed status=0x%x\n", sts);
> +		return false;
> +	}
> +
> +	pci_iounmap(parent, base);
> +	return true;
> +}
> +
> +static void ls2k_bmc_restore_bridge_pci_data(struct pci_dev *parent, struct ls2k_bmc_pdata *priv)
> +{
> +	int base, i = 0;
> +
> +	pci_write_config_dword(parent, PCI_COMMAND, priv->bridge_pci_data.pci_command);
> +
> +	for (base = PCI_BASE_ADDRESS_0; base <= PCI_BASE_ADDRESS_5; base += 4, i++)
> +		pci_write_config_dword(parent, base, priv->bridge_pci_data.base_address[i]);
> +
> +	pci_write_config_dword(parent, PCI_ROM_ADDRESS, priv->bridge_pci_data.rom_addreess);
> +	pci_write_config_dword(parent, PCI_INTERRUPT_LINE, priv->bridge_pci_data.interrupt_line);
> +
> +	pci_write_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_LO,
> +			       priv->bridge_pci_data.msi_lo);
> +	pci_write_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_HI,
> +			       priv->bridge_pci_data.msi_hi);
> +	pci_write_config_dword(parent, parent->pcie_cap + PCI_EXP_DEVCTL,
> +			       priv->bridge_pci_data.devctl);
> +	pci_write_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCAP,
> +			       priv->bridge_pci_data.linkcap);
> +	pci_write_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCTL,
> +			       priv->bridge_pci_data.linkctl_sts);
> +
> +	pci_write_config_dword(parent, LS7A_GEN2_CTL, priv->bridge_pci_data.gen2_ctrl);
> +	pci_write_config_dword(parent, LS7A_SYMBOL_TIMER, priv->bridge_pci_data.symbol_timer);
> +}
> +
> +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;
> +	u32 i;
> +
> +	/*
> +	 * Clear the bus, io and mem resources of the PCI-E bridge to zero, so that
> +	 * the processor can not access the LS2K PCI-E port, to avoid crashing due to
> +	 * the lack of return signal from accessing the LS2K PCI-E port.
> +	 */
> +	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);
> +
> +	/*
> +	 * When the LS2K BMC is reset, the LS7A PCI-E port is also reset, and its PCI
> +	 * BAR0 register is cleared. Due to the time gap between the GPIO interrupt
> +	 * generation and the LS2K BMC reset, the LS7A PCI BAR0 register is read to
> +	 * determine whether the reset has begun.
> +	 */
> +	for (i = LS7A_BAR0_CHECK_MAX_TIMES; i > 0 ; i--) {
> +		if (!ls2k_bmc_bar0_addr_is_set(parent))
> +			break;
> +		mdelay(1);
> +	};
> +
> +	if (i == 0)
> +		return false;
> +
> +	ls2k_bmc_restore_bridge_pci_data(parent, priv);
> +
> +	/* Check if PCI-E is connected */
> +	if (!ls2k_bmc_pcie_is_connected(parent, priv))
> +		return false;
> +
> +	/* Waiting for U-Boot and DDR ready */
> +	mdelay(LS2K_BMC_RESET_WAIT_TIME);
> +	if (!ls2k_bmc_bar0_addr_is_set(parent))
> +		return false;
> +
> +	/* Restore LS2K BMC PCI-E config data */
> +	pci_write_config_dword(pdev, PCI_COMMAND, priv->bmc_pci_data.pci_command);
> +	pci_write_config_dword(pdev, PCI_BASE_ADDRESS_0, priv->bmc_pci_data.base_address0);
> +	pci_write_config_dword(pdev, PCI_INTERRUPT_LINE, priv->bmc_pci_data.interrupt_line);
> +
> +	return 0;
> +}
> +
> +static void ls2k_bmc_events_fn(struct work_struct *work)
> +{
> +	struct ls2k_bmc_pdata *priv = container_of(work, struct ls2k_bmc_pdata, bmc_reset_work);
> +
> +	/*
> +	 * The PCI-E is lost when the BMC resets, at which point access to the PCI-E
> +	 * from other CPUs is suspended to prevent a crash.
> +	 */
> +	stop_machine(ls2k_bmc_recover_pci_data, priv, NULL);
> +
> +#ifdef CONFIG_VT

#ifery in C-files is generally frowned upon.

Is the any pieces of run-time data you can use instead?

Or a stub which culminated in a no-op if !CONFIG_VT?

> +	/* Re-push the display due to previous PCI-E loss. */
> +	set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> +#endif
> +}
> +
> +static irqreturn_t ls2k_bmc_interrupt(int irq, void *arg)
> +{
> +	struct ls2k_bmc_pdata *priv = arg;
> +	static unsigned long last_jiffies;
> +
> +	if (system_state != SYSTEM_RUNNING)
> +		return IRQ_HANDLED;
> +
> +	/* Skip interrupt in LS2K_BMC_INT_INTERVAL */
> +	if (time_after(jiffies, last_jiffies + LS2K_BMC_INT_INTERVAL)) {
> +		schedule_work(&priv->bmc_reset_work);
> +		last_jiffies = jiffies;
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/*
> + * Saves the BMC parent device (LS7A) and its own PCI configuration space registers
> + * that need to be restored after BMC reset.
> + */
> +static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> +{
> +	struct pci_dev *parent = pdev->bus->self;
> +	int base, i = 0;
> +
> +	pci_read_config_dword(parent, PCI_COMMAND, &priv->bridge_pci_data.pci_command);
> +
> +	for (base = PCI_BASE_ADDRESS_0; base <= PCI_BASE_ADDRESS_5; base += 4, i++)
> +		pci_read_config_dword(parent, base, &priv->bridge_pci_data.base_address[i]);
> +
> +	pci_read_config_dword(parent, PCI_ROM_ADDRESS, &priv->bridge_pci_data.rom_addreess);
> +	pci_read_config_dword(parent, PCI_INTERRUPT_LINE, &priv->bridge_pci_data.interrupt_line);
> +
> +	pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_LO,
> +			      &priv->bridge_pci_data.msi_lo);
> +	pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_HI,
> +			      &priv->bridge_pci_data.msi_hi);
> +
> +	pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_DEVCTL,
> +			      &priv->bridge_pci_data.devctl);
> +	pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCAP,
> +			      &priv->bridge_pci_data.linkcap);
> +	pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCTL,
> +			      &priv->bridge_pci_data.linkctl_sts);
> +
> +	pci_read_config_dword(parent, LS7A_GEN2_CTL, &priv->bridge_pci_data.gen2_ctrl);
> +	priv->bridge_pci_data.gen2_ctrl |= FIELD_PREP(LS7A_GEN2_SPEED_CHANG, 0x1)
> +					| FIELD_PREP(LS7A_CONF_PHY_TX, 0x0);
> +
> +	pci_read_config_dword(parent, LS7A_SYMBOL_TIMER, &priv->bridge_pci_data.symbol_timer);
> +	priv->bridge_pci_data.symbol_timer |= LS7A_MASK_LEN_MATCH;
> +
> +	pci_read_config_dword(pdev, PCI_COMMAND, &priv->bmc_pci_data.pci_command);
> +	pci_read_config_dword(pdev, PCI_BASE_ADDRESS_0, &priv->bmc_pci_data.base_address0);
> +	pci_read_config_dword(pdev, PCI_INTERRUPT_LINE, &priv->bmc_pci_data.interrupt_line);
> +}
> +
> +static int ls2k_bmc_pdata_initial(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> +{
> +	int gsi = 16 + (LS2K_BMC_RESET_GPIO & 7);
> +	void __iomem *gpio_base;
> +	int irq, ret;
> +
> +	ls2k_bmc_save_pci_data(pdev, priv);
> +
> +	INIT_WORK(&priv->bmc_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 PCI-E request_irq(%d) failed\n", pdev->irq);

Please don't use function names in error messages.

Make them human readable inclusive of non-kernel engineers.

> +		return ret;
> +	}
> +
> +	/*
> +	 * Since gpio_chip->to_irq is not implemented in the Loongson-3 GPIO driver,
> +	 * acpi_register_gsi() is used to obtain the GPIO irq. The GPIO interrupt is a

"IRQ"

> +	 * watchdog interrupt that is triggered when the BMC resets.
> +	 */
> +	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(LS2K_BMC_RESET_GPIO),
> +	       gpio_base + LOONGSON_GPIO_OEN);
> +	writel(readl(gpio_base + LOONGSON_GPIO_FUNC) & ~BIT(LS2K_BMC_RESET_GPIO),
> +	       gpio_base + LOONGSON_GPIO_FUNC);
> +	writel(readl(gpio_base + LOONGSON_GPIO_INTPOL) & ~BIT(LS2K_BMC_RESET_GPIO),
> +	       gpio_base + LOONGSON_GPIO_INTPOL);
> +	writel(readl(gpio_base + LOONGSON_GPIO_INTEN) | BIT(LS2K_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;
> +}
> +
>  /*
>   * Currently the Loongson-2K 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.
> @@ -101,6 +416,7 @@ static int ls2k_bmc_parse_mode(struct pci_dev *pdev, struct simplefb_platform_da
>  static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  {
>  	struct simplefb_platform_data pd;
> +	struct ls2k_bmc_pdata *priv;
>  	resource_size_t base;
>  	int ret;
>  
> @@ -108,6 +424,18 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  	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);

priv (ddata) already contains dev - you don't need both.

> +	if (ret)
> +		goto disable_pci;
> +
>  	ret = ls2k_bmc_parse_mode(dev, &pd);
>  	if (ret)
>  		goto disable_pci;
> -- 
> 2.47.1
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH v7 1/3] mfd: ls2kbmc: Introduce Loongson-2K BMC core driver

[Binbin Zhou]

Hi Lee:

Thanks for your review.

On Thu, Jul 10, 2025 at 5:56 PM Lee Jones <lee@kernel.org> wrote:
>
> On Fri, 04 Jul 2025, Binbin Zhou 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-3 CPU. It supports multiple sub-devices like DRM and IPMI.
> >
> > Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> > Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> > Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> > Acked-by: Corey Minyard <corey@minyard.net>
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > ---
> >  MAINTAINERS                 |   6 ++
> >  drivers/mfd/Kconfig         |  13 +++
> >  drivers/mfd/Makefile        |   2 +
> >  drivers/mfd/ls2k-bmc-core.c | 156 ++++++++++++++++++++++++++++++++++++
> >  4 files changed, 177 insertions(+)
> >  create mode 100644 drivers/mfd/ls2k-bmc-core.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 0d053c45f7f9..4eb0f7b69d35 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -14199,6 +14199,12 @@ S:   Maintained
> >  F:   Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> >  F:   drivers/thermal/loongson2_thermal.c
> >
> > +LOONGSON-2K Board Management Controller (BMC) DRIVER
> > +M:   Binbin Zhou <zhoubinbin@loongson.cn>
> > +M:   Chong Qiao <qiaochong@loongson.cn>
> > +S:   Maintained
> > +F:   drivers/mfd/ls2k-bmc-core.c
> > +
> >  LOONGSON EDAC DRIVER
> >  M:   Zhao Qunqin <zhaoqunqin@loongson.cn>
> >  L:   linux-edac@vger.kernel.org
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index c635790afa75..47cc8ea9d2ef 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -2400,6 +2400,19 @@ config MFD_INTEL_M10_BMC_PMCI
> >         additional drivers must be enabled in order to use the functionality
> >         of the device.
> >
> > +config MFD_LS2K_BMC_CORE
> > +     bool "Loongson-2K Board Management Controller Support"
> > +     depends on PCI && ACPI_GENERIC_GSI
> > +     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 display and IPMI. This driver provides common
> > +       support for accessing the devices.
> > +
> > +       The display is enabled by default in the driver, while the IPMI interface
> > +       is enabled independently through the IPMI_LS2K option in the IPMI section.
> > +
> >  config MFD_QNAP_MCU
> >       tristate "QNAP microcontroller unit core driver"
> >       depends on SERIAL_DEV_BUS
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index ca351cb0ddcc..675b4ec6ef4c 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -284,6 +284,8 @@ obj-$(CONFIG_MFD_INTEL_M10_BMC_CORE)   += intel-m10-bmc-core.o
> >  obj-$(CONFIG_MFD_INTEL_M10_BMC_SPI)    += intel-m10-bmc-spi.o
> >  obj-$(CONFIG_MFD_INTEL_M10_BMC_PMCI)   += intel-m10-bmc-pmci.o
> >
> > +obj-$(CONFIG_MFD_LS2K_BMC_CORE)              += ls2k-bmc-core.o
> > +
> >  obj-$(CONFIG_MFD_ATC260X)    += atc260x-core.o
> >  obj-$(CONFIG_MFD_ATC260X_I2C)        += atc260x-i2c.o
> >
> > diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> > new file mode 100644
> > index 000000000000..50d560a4611c
> > --- /dev/null
> > +++ b/drivers/mfd/ls2k-bmc-core.c
> > @@ -0,0 +1,156 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Loongson-2K Board Management Controller (BMC) Core Driver.
> > + *
> > + * Copyright (C) 2024-2025 Loongson Technology Corporation Limited.
> > + *
> > + * Authors:
> > + *   Chong Qiao <qiaochong@loongson.cn>
> > + *   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>
> > +
> > +/* LS2K BMC resources */
> > +#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-2K 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_parse_mode(struct pci_dev *pdev, struct simplefb_platform_data *pd)
> > +{
> > +     char *mode;
> > +     int depth, ret;
> > +
> > +     /* 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;
> > +
> > +     /* 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;
> > +}
> > +
> > +static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
> > +{
> > +     struct simplefb_platform_data pd;
> > +     resource_size_t base;
> > +     int ret;
> > +
> > +     ret = pci_enable_device(dev);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = ls2k_bmc_parse_mode(dev, &pd);
> > +     if (ret)
> > +             goto disable_pci;
> > +
> > +     ls2k_bmc_cells[0].platform_data = &pd;
> > +     ls2k_bmc_cells[0].pdata_size = sizeof(pd);
>
> This is fragile.
>
> Please identify the elements in ls2k_bmc_cells and use it to index here.
>
> See: `static struct mfd_cell as3711_subdevs`

How about this:

enum {
        LS2K_BMC_DISPLAY,
        LS2k_BMC_IPMI0,
        LS2k_BMC_IPMI1,
        LS2k_BMC_IPMI2,
        LS2k_BMC_IPMI3,
        LS2k_BMC_IPMI4,
};

static struct mfd_cell ls2k_bmc_cells[] = {
        [LS2K_BMC_DISPLAY] = {
                .name = "simple-framebuffer",
                .num_resources = ARRAY_SIZE(ls2k_display_resources),
                .resources = ls2k_display_resources
        },
        [LS2k_BMC_IPMI0] = {
                .name = "ls2k-ipmi-si",
                .num_resources = ARRAY_SIZE(ls2k_ipmi0_resources),
                .resources = ls2k_ipmi0_resources
        },
        [LS2k_BMC_IPMI1] = {
                .name = "ls2k-ipmi-si",
                .num_resources = ARRAY_SIZE(ls2k_ipmi1_resources),
                .resources = ls2k_ipmi1_resources
        },
        [LS2k_BMC_IPMI2] = {
                .name = "ls2k-ipmi-si",
                .num_resources = ARRAY_SIZE(ls2k_ipmi2_resources),
                .resources = ls2k_ipmi2_resources
        },
        [LS2k_BMC_IPMI3] = {
                .name = "ls2k-ipmi-si",
                .num_resources = ARRAY_SIZE(ls2k_ipmi3_resources),
                .resources = ls2k_ipmi3_resources
        },
        [LS2k_BMC_IPMI4] = {
                .name = "ls2k-ipmi-si",
                .num_resources = ARRAY_SIZE(ls2k_ipmi4_resources),
                .resources = ls2k_ipmi4_resources
        },
};

and

        ls2k_bmc_cells[LS2K_BMC_DISPLAY].platform_data = &pd;
        ls2k_bmc_cells[LS2K_BMC_DISPLAY].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, "Failed to removed firmware framebuffers: %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 Board Management Controller (BMC) Core driver");
> > +MODULE_AUTHOR("Loongson Technology Corporation Limited");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.47.1
> >
>
> --
> Lee Jones [李琼斯]


--
Thanks.
Binbin

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

[Binbin Zhou]

Hi Lee:

Thanks for your review.

On Thu, Jul 10, 2025 at 6:03 PM Lee Jones <lee@kernel.org> wrote:
>
> On Fri, 04 Jul 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.
> >
> > Quick overview of the entire LS2K BMC reset process:
> >
> > There are two types of reset methods: soft reset (BMC-initiated reboot
> > of IPMI reset command) and BMC watchdog reset (watchdog timeout).
> >
> > First, regardless of the method, an interrupt is generated (PCIe interrupt
> > for soft reset/GPIO interrupt for watchdog reset);
> >
> > Second, during the interrupt process, the system enters bmc_reset_work,
> > clears the bus/IO/mem resources of the LS7A PCI-E bridge, waits for the BMC
> > reset to begin, then restores the parent device's PCI configuration space,
> > waits for the BMC reset to complete, and finally restores the BMC PCI
> > configuration space.
> >
> > Display restoration occurs last.
> >
> > Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> > Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> > Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> > Acked-by: Corey Minyard <corey@minyard.net>
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > ---
> >  drivers/mfd/ls2k-bmc-core.c | 328 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 328 insertions(+)
> >
> > diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> > index 50d560a4611c..1ae673f6a196 100644
> > --- a/drivers/mfd/ls2k-bmc-core.c
> > +++ b/drivers/mfd/ls2k-bmc-core.c
> > @@ -10,8 +10,12 @@
> >   */
> >
> >  #include <linux/aperture.h>
> > +#include <linux/bitfield.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>
> > @@ -19,6 +23,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>
> >
> >  /* LS2K BMC resources */
> >  #define LS2K_DISPLAY_RES_START               (SZ_16M + SZ_2M)
> > @@ -29,6 +35,48 @@
> >  #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 LS7A_PCI_CFG_SIZE            0x100
> > +
> > +/* LS7A bridge registers */
> > +#define LS7A_PCIE_PORT_CTL0          0x0
> > +#define LS7A_PCIE_PORT_STS1          0xC
> > +#define LS7A_GEN2_CTL                        0x80C
> > +#define LS7A_SYMBOL_TIMER            0x71C
> > +
> > +/* Bits of LS7A_PCIE_PORT_CTL0 */
> > +#define LS2K_BMC_PCIE_LTSSM_ENABLE   BIT(3)
> > +
> > +/* Bits of LS7A_PCIE_PORT_STS1 */
> > +#define LS2K_BMC_PCIE_LTSSM_STS              GENMASK(5, 0)
> > +#define LS2K_BMC_PCIE_CONNECTED              0x11
> > +
> > +#define LS2K_BMC_PCIE_DELAY_US               1000
> > +#define LS2K_BMC_PCIE_TIMEOUT_US     1000000
> > +
> > +/* Bits of LS7A_GEN2_CTL */
> > +#define LS7A_GEN2_SPEED_CHANG                BIT(17)
> > +#define LS7A_CONF_PHY_TX             BIT(18)
> > +
> > +/* Bits of LS7A_SYMBOL_TIMER */
> > +#define LS7A_MASK_LEN_MATCH          BIT(26)
> > +
> > +/* Interval between interruptions */
> > +#define LS2K_BMC_INT_INTERVAL                (60 * HZ)
> > +
> > +/* Maximum time to wait for U-Boot and DDR to be ready with ms. */
> > +#define LS2K_BMC_RESET_WAIT_TIME     10000
> > +
> > +/* It's an experience value */
> > +#define LS7A_BAR0_CHECK_MAX_TIMES    2000
> > +
> > +#define LS2K_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
> > +
> >  static struct resource ls2k_display_resources[] = {
> >       DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
> >  };
> > @@ -62,6 +110,273 @@ static struct mfd_cell ls2k_bmc_cells[] = {
> >       MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
> >  };
> >
> > +/* Index of the BMC PCI configuration space to be restored at BMC reset. */
> > +struct ls2k_bmc_pci_data {
> > +     u32 pci_command;
> > +     u32 base_address0;
> > +     u32 interrupt_line;
> > +};
> > +
> > +/* Index of the parent PCI configuration space to be restored at BMC reset. */
> > +struct ls2k_bmc_bridge_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;
> > +};
> > +
> > +struct ls2k_bmc_pdata {
> > +     struct device *dev;
> > +     struct work_struct bmc_reset_work;
> > +     struct ls2k_bmc_pci_data bmc_pci_data;
> > +     struct ls2k_bmc_bridge_pci_data bridge_pci_data;
> > +};
> > +
> > +static bool ls2k_bmc_bar0_addr_is_set(struct pci_dev *ppdev)
>
> Nit: This is usually called pdev.

OK.
>
> > +{
> > +     u32 addr;
> > +
> > +     pci_read_config_dword(ppdev, PCI_BASE_ADDRESS_0, &addr);
> > +
> > +     return addr & PCI_BASE_ADDRESS_MEM_MASK ? true : false;
> > +}
> > +
> > +static bool ls2k_bmc_pcie_is_connected(struct pci_dev *parent, struct ls2k_bmc_pdata *priv)
>
> Nit: Rename priv to ddata.

OK. I will check and rename all `priv` to `ddata`.
>
> > +{
> > +     void __iomem *base;
> > +     int sts, ret;
> > +
> > +     base = pci_iomap(parent, 0, LS7A_PCI_CFG_SIZE);
> > +     if (!base)
> > +             return false;
> > +
> > +     writel(readl(base + LS7A_PCIE_PORT_CTL0) | LS2K_BMC_PCIE_LTSSM_ENABLE,
> > +            base + LS7A_PCIE_PORT_CTL0);
> > +
> > +     ret = readl_poll_timeout_atomic(base + LS7A_PCIE_PORT_STS1, sts,
> > +                                     (sts & LS2K_BMC_PCIE_LTSSM_STS) == LS2K_BMC_PCIE_CONNECTED,
> > +                                     LS2K_BMC_PCIE_DELAY_US, LS2K_BMC_PCIE_TIMEOUT_US);
> > +     if (ret) {
> > +             pci_iounmap(parent, base);
> > +             dev_err(priv->dev, "PCIE train failed status=0x%x\n", sts);
> > +             return false;
> > +     }
> > +
> > +     pci_iounmap(parent, base);
> > +     return true;
> > +}
> > +
> > +static void ls2k_bmc_restore_bridge_pci_data(struct pci_dev *parent, struct ls2k_bmc_pdata *priv)
> > +{
> > +     int base, i = 0;
> > +
> > +     pci_write_config_dword(parent, PCI_COMMAND, priv->bridge_pci_data.pci_command);
> > +
> > +     for (base = PCI_BASE_ADDRESS_0; base <= PCI_BASE_ADDRESS_5; base += 4, i++)
> > +             pci_write_config_dword(parent, base, priv->bridge_pci_data.base_address[i]);
> > +
> > +     pci_write_config_dword(parent, PCI_ROM_ADDRESS, priv->bridge_pci_data.rom_addreess);
> > +     pci_write_config_dword(parent, PCI_INTERRUPT_LINE, priv->bridge_pci_data.interrupt_line);
> > +
> > +     pci_write_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_LO,
> > +                            priv->bridge_pci_data.msi_lo);
> > +     pci_write_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_HI,
> > +                            priv->bridge_pci_data.msi_hi);
> > +     pci_write_config_dword(parent, parent->pcie_cap + PCI_EXP_DEVCTL,
> > +                            priv->bridge_pci_data.devctl);
> > +     pci_write_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCAP,
> > +                            priv->bridge_pci_data.linkcap);
> > +     pci_write_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCTL,
> > +                            priv->bridge_pci_data.linkctl_sts);
> > +
> > +     pci_write_config_dword(parent, LS7A_GEN2_CTL, priv->bridge_pci_data.gen2_ctrl);
> > +     pci_write_config_dword(parent, LS7A_SYMBOL_TIMER, priv->bridge_pci_data.symbol_timer);
> > +}
> > +
> > +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;
> > +     u32 i;
> > +
> > +     /*
> > +      * Clear the bus, io and mem resources of the PCI-E bridge to zero, so that
> > +      * the processor can not access the LS2K PCI-E port, to avoid crashing due to
> > +      * the lack of return signal from accessing the LS2K PCI-E port.
> > +      */
> > +     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);
> > +
> > +     /*
> > +      * When the LS2K BMC is reset, the LS7A PCI-E port is also reset, and its PCI
> > +      * BAR0 register is cleared. Due to the time gap between the GPIO interrupt
> > +      * generation and the LS2K BMC reset, the LS7A PCI BAR0 register is read to
> > +      * determine whether the reset has begun.
> > +      */
> > +     for (i = LS7A_BAR0_CHECK_MAX_TIMES; i > 0 ; i--) {
> > +             if (!ls2k_bmc_bar0_addr_is_set(parent))
> > +                     break;
> > +             mdelay(1);
> > +     };
> > +
> > +     if (i == 0)
> > +             return false;
> > +
> > +     ls2k_bmc_restore_bridge_pci_data(parent, priv);
> > +
> > +     /* Check if PCI-E is connected */
> > +     if (!ls2k_bmc_pcie_is_connected(parent, priv))
> > +             return false;
> > +
> > +     /* Waiting for U-Boot and DDR ready */
> > +     mdelay(LS2K_BMC_RESET_WAIT_TIME);
> > +     if (!ls2k_bmc_bar0_addr_is_set(parent))
> > +             return false;
> > +
> > +     /* Restore LS2K BMC PCI-E config data */
> > +     pci_write_config_dword(pdev, PCI_COMMAND, priv->bmc_pci_data.pci_command);
> > +     pci_write_config_dword(pdev, PCI_BASE_ADDRESS_0, priv->bmc_pci_data.base_address0);
> > +     pci_write_config_dword(pdev, PCI_INTERRUPT_LINE, priv->bmc_pci_data.interrupt_line);
> > +
> > +     return 0;
> > +}
> > +
> > +static void ls2k_bmc_events_fn(struct work_struct *work)
> > +{
> > +     struct ls2k_bmc_pdata *priv = container_of(work, struct ls2k_bmc_pdata, bmc_reset_work);
> > +
> > +     /*
> > +      * The PCI-E is lost when the BMC resets, at which point access to the PCI-E
> > +      * from other CPUs is suspended to prevent a crash.
> > +      */
> > +     stop_machine(ls2k_bmc_recover_pci_data, priv, NULL);
> > +
> > +#ifdef CONFIG_VT
>
> #ifery in C-files is generally frowned upon.
>
> Is the any pieces of run-time data you can use instead?
>
> Or a stub which culminated in a no-op if !CONFIG_VT?

Emm, I'm not sure if I understand correctly, but is the following way suitable?

        if (IS_ENABLED(CONFIG_VT))
                /* Re-push the display due to previous PCI-E loss. */
                set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));

>
> > +     /* Re-push the display due to previous PCI-E loss. */
> > +     set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> > +#endif
> > +}
> > +
> > +static irqreturn_t ls2k_bmc_interrupt(int irq, void *arg)
> > +{
> > +     struct ls2k_bmc_pdata *priv = arg;
> > +     static unsigned long last_jiffies;
> > +
> > +     if (system_state != SYSTEM_RUNNING)
> > +             return IRQ_HANDLED;
> > +
> > +     /* Skip interrupt in LS2K_BMC_INT_INTERVAL */
> > +     if (time_after(jiffies, last_jiffies + LS2K_BMC_INT_INTERVAL)) {
> > +             schedule_work(&priv->bmc_reset_work);
> > +             last_jiffies = jiffies;
> > +     }
> > +
> > +     return IRQ_HANDLED;
> > +}
> > +
> > +/*
> > + * Saves the BMC parent device (LS7A) and its own PCI configuration space registers
> > + * that need to be restored after BMC reset.
> > + */
> > +static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > +{
> > +     struct pci_dev *parent = pdev->bus->self;
> > +     int base, i = 0;
> > +
> > +     pci_read_config_dword(parent, PCI_COMMAND, &priv->bridge_pci_data.pci_command);
> > +
> > +     for (base = PCI_BASE_ADDRESS_0; base <= PCI_BASE_ADDRESS_5; base += 4, i++)
> > +             pci_read_config_dword(parent, base, &priv->bridge_pci_data.base_address[i]);
> > +
> > +     pci_read_config_dword(parent, PCI_ROM_ADDRESS, &priv->bridge_pci_data.rom_addreess);
> > +     pci_read_config_dword(parent, PCI_INTERRUPT_LINE, &priv->bridge_pci_data.interrupt_line);
> > +
> > +     pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_LO,
> > +                           &priv->bridge_pci_data.msi_lo);
> > +     pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_HI,
> > +                           &priv->bridge_pci_data.msi_hi);
> > +
> > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_DEVCTL,
> > +                           &priv->bridge_pci_data.devctl);
> > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCAP,
> > +                           &priv->bridge_pci_data.linkcap);
> > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCTL,
> > +                           &priv->bridge_pci_data.linkctl_sts);
> > +
> > +     pci_read_config_dword(parent, LS7A_GEN2_CTL, &priv->bridge_pci_data.gen2_ctrl);
> > +     priv->bridge_pci_data.gen2_ctrl |= FIELD_PREP(LS7A_GEN2_SPEED_CHANG, 0x1)
> > +                                     | FIELD_PREP(LS7A_CONF_PHY_TX, 0x0);
> > +
> > +     pci_read_config_dword(parent, LS7A_SYMBOL_TIMER, &priv->bridge_pci_data.symbol_timer);
> > +     priv->bridge_pci_data.symbol_timer |= LS7A_MASK_LEN_MATCH;
> > +
> > +     pci_read_config_dword(pdev, PCI_COMMAND, &priv->bmc_pci_data.pci_command);
> > +     pci_read_config_dword(pdev, PCI_BASE_ADDRESS_0, &priv->bmc_pci_data.base_address0);
> > +     pci_read_config_dword(pdev, PCI_INTERRUPT_LINE, &priv->bmc_pci_data.interrupt_line);
> > +}
> > +
> > +static int ls2k_bmc_pdata_initial(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > +{
> > +     int gsi = 16 + (LS2K_BMC_RESET_GPIO & 7);
> > +     void __iomem *gpio_base;
> > +     int irq, ret;
> > +
> > +     ls2k_bmc_save_pci_data(pdev, priv);
> > +
> > +     INIT_WORK(&priv->bmc_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 PCI-E request_irq(%d) failed\n", pdev->irq);
>
> Please don't use function names in error messages.
>
> Make them human readable inclusive of non-kernel engineers.

How about:

dev_err(ddata->dev, "Failed to request LS2KBMC PCI-E IRQ %d.\n", pdev->irq);

also, the error message regarding GPIO IRQ is as follows:

dev_err(ddata->dev, "Failed to request LS2KBMC GPIO IRQ %d.\n", irq);
>
> > +             return ret;
> > +     }
> > +
> > +     /*
> > +      * Since gpio_chip->to_irq is not implemented in the Loongson-3 GPIO driver,
> > +      * acpi_register_gsi() is used to obtain the GPIO irq. The GPIO interrupt is a
>
> "IRQ"
>
> > +      * watchdog interrupt that is triggered when the BMC resets.
> > +      */
> > +     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(LS2K_BMC_RESET_GPIO),
> > +            gpio_base + LOONGSON_GPIO_OEN);
> > +     writel(readl(gpio_base + LOONGSON_GPIO_FUNC) & ~BIT(LS2K_BMC_RESET_GPIO),
> > +            gpio_base + LOONGSON_GPIO_FUNC);
> > +     writel(readl(gpio_base + LOONGSON_GPIO_INTPOL) & ~BIT(LS2K_BMC_RESET_GPIO),
> > +            gpio_base + LOONGSON_GPIO_INTPOL);
> > +     writel(readl(gpio_base + LOONGSON_GPIO_INTEN) | BIT(LS2K_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;
> > +}
> > +
> >  /*
> >   * Currently the Loongson-2K 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.
> > @@ -101,6 +416,7 @@ static int ls2k_bmc_parse_mode(struct pci_dev *pdev, struct simplefb_platform_da
> >  static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
> >  {
> >       struct simplefb_platform_data pd;
> > +     struct ls2k_bmc_pdata *priv;
> >       resource_size_t base;
> >       int ret;
> >
> > @@ -108,6 +424,18 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
> >       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);
>
> priv (ddata) already contains dev - you don't need both.

Yes, we just pass priv(ddata), and

struct pci_dev *pdev = to_pci_dev(ddata->dev);

>
> > +     if (ret)
> > +             goto disable_pci;
> > +
> >       ret = ls2k_bmc_parse_mode(dev, &pd);
> >       if (ret)
> >               goto disable_pci;
> > --
> > 2.47.1
> >
>
> --
> Lee Jones [李琼斯]

--
Thanks.
Binbin

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

[Lee Jones]

On Fri, 11 Jul 2025, Binbin Zhou wrote:

> Hi Lee:
> 
> Thanks for your review.
> 
> On Thu, Jul 10, 2025 at 6:03 PM Lee Jones <lee@kernel.org> wrote:
> >
> > On Fri, 04 Jul 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.
> > >
> > > Quick overview of the entire LS2K BMC reset process:
> > >
> > > There are two types of reset methods: soft reset (BMC-initiated reboot
> > > of IPMI reset command) and BMC watchdog reset (watchdog timeout).
> > >
> > > First, regardless of the method, an interrupt is generated (PCIe interrupt
> > > for soft reset/GPIO interrupt for watchdog reset);
> > >
> > > Second, during the interrupt process, the system enters bmc_reset_work,
> > > clears the bus/IO/mem resources of the LS7A PCI-E bridge, waits for the BMC
> > > reset to begin, then restores the parent device's PCI configuration space,
> > > waits for the BMC reset to complete, and finally restores the BMC PCI
> > > configuration space.
> > >
> > > Display restoration occurs last.
> > >
> > > Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> > > Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> > > Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> > > Acked-by: Corey Minyard <corey@minyard.net>
> > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > ---
> > >  drivers/mfd/ls2k-bmc-core.c | 328 ++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 328 insertions(+)
> > >
> > > diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> > > index 50d560a4611c..1ae673f6a196 100644
> > > --- a/drivers/mfd/ls2k-bmc-core.c
> > > +++ b/drivers/mfd/ls2k-bmc-core.c
> > > @@ -10,8 +10,12 @@
> > >   */
> > >
> > >  #include <linux/aperture.h>
> > > +#include <linux/bitfield.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>
> > > @@ -19,6 +23,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>
> > >
> > >  /* LS2K BMC resources */
> > >  #define LS2K_DISPLAY_RES_START               (SZ_16M + SZ_2M)
> > > @@ -29,6 +35,48 @@
> > >  #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 LS7A_PCI_CFG_SIZE            0x100
> > > +
> > > +/* LS7A bridge registers */
> > > +#define LS7A_PCIE_PORT_CTL0          0x0
> > > +#define LS7A_PCIE_PORT_STS1          0xC
> > > +#define LS7A_GEN2_CTL                        0x80C
> > > +#define LS7A_SYMBOL_TIMER            0x71C
> > > +
> > > +/* Bits of LS7A_PCIE_PORT_CTL0 */
> > > +#define LS2K_BMC_PCIE_LTSSM_ENABLE   BIT(3)
> > > +
> > > +/* Bits of LS7A_PCIE_PORT_STS1 */
> > > +#define LS2K_BMC_PCIE_LTSSM_STS              GENMASK(5, 0)
> > > +#define LS2K_BMC_PCIE_CONNECTED              0x11
> > > +
> > > +#define LS2K_BMC_PCIE_DELAY_US               1000
> > > +#define LS2K_BMC_PCIE_TIMEOUT_US     1000000
> > > +
> > > +/* Bits of LS7A_GEN2_CTL */
> > > +#define LS7A_GEN2_SPEED_CHANG                BIT(17)
> > > +#define LS7A_CONF_PHY_TX             BIT(18)
> > > +
> > > +/* Bits of LS7A_SYMBOL_TIMER */
> > > +#define LS7A_MASK_LEN_MATCH          BIT(26)
> > > +
> > > +/* Interval between interruptions */
> > > +#define LS2K_BMC_INT_INTERVAL                (60 * HZ)
> > > +
> > > +/* Maximum time to wait for U-Boot and DDR to be ready with ms. */
> > > +#define LS2K_BMC_RESET_WAIT_TIME     10000
> > > +
> > > +/* It's an experience value */
> > > +#define LS7A_BAR0_CHECK_MAX_TIMES    2000
> > > +
> > > +#define LS2K_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
> > > +
> > >  static struct resource ls2k_display_resources[] = {
> > >       DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
> > >  };
> > > @@ -62,6 +110,273 @@ static struct mfd_cell ls2k_bmc_cells[] = {
> > >       MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
> > >  };
> > >
> > > +/* Index of the BMC PCI configuration space to be restored at BMC reset. */
> > > +struct ls2k_bmc_pci_data {
> > > +     u32 pci_command;
> > > +     u32 base_address0;
> > > +     u32 interrupt_line;
> > > +};
> > > +
> > > +/* Index of the parent PCI configuration space to be restored at BMC reset. */
> > > +struct ls2k_bmc_bridge_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;
> > > +};
> > > +
> > > +struct ls2k_bmc_pdata {
> > > +     struct device *dev;
> > > +     struct work_struct bmc_reset_work;
> > > +     struct ls2k_bmc_pci_data bmc_pci_data;
> > > +     struct ls2k_bmc_bridge_pci_data bridge_pci_data;
> > > +};
> > > +
> > > +static bool ls2k_bmc_bar0_addr_is_set(struct pci_dev *ppdev)
> >
> > Nit: This is usually called pdev.
> 
> OK.

Snip things you agree with please.

[...]

> > > +static void ls2k_bmc_events_fn(struct work_struct *work)
> > > +{
> > > +     struct ls2k_bmc_pdata *priv = container_of(work, struct ls2k_bmc_pdata, bmc_reset_work);
> > > +
> > > +     /*
> > > +      * The PCI-E is lost when the BMC resets, at which point access to the PCI-E
> > > +      * from other CPUs is suspended to prevent a crash.
> > > +      */
> > > +     stop_machine(ls2k_bmc_recover_pci_data, priv, NULL);
> > > +
> > > +#ifdef CONFIG_VT
> >
> > #ifery in C-files is generally frowned upon.
> >
> > Is the any pieces of run-time data you can use instead?
> >
> > Or a stub which culminated in a no-op if !CONFIG_VT?
> 
> Emm, I'm not sure if I understand correctly, but is the following way suitable?
> 
>         if (IS_ENABLED(CONFIG_VT))

It's better than #if, but even better would be a stub in a header file.

>                 /* Re-push the display due to previous PCI-E loss. */
>                 set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> 
> >
> > > +     /* Re-push the display due to previous PCI-E loss. */
> > > +     set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> > > +#endif
> > > +}
> > > +
> > > +static irqreturn_t ls2k_bmc_interrupt(int irq, void *arg)
> > > +{
> > > +     struct ls2k_bmc_pdata *priv = arg;
> > > +     static unsigned long last_jiffies;
> > > +
> > > +     if (system_state != SYSTEM_RUNNING)
> > > +             return IRQ_HANDLED;
> > > +
> > > +     /* Skip interrupt in LS2K_BMC_INT_INTERVAL */
> > > +     if (time_after(jiffies, last_jiffies + LS2K_BMC_INT_INTERVAL)) {
> > > +             schedule_work(&priv->bmc_reset_work);
> > > +             last_jiffies = jiffies;
> > > +     }
> > > +
> > > +     return IRQ_HANDLED;
> > > +}
> > > +
> > > +/*
> > > + * Saves the BMC parent device (LS7A) and its own PCI configuration space registers
> > > + * that need to be restored after BMC reset.
> > > + */
> > > +static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > > +{
> > > +     struct pci_dev *parent = pdev->bus->self;
> > > +     int base, i = 0;
> > > +
> > > +     pci_read_config_dword(parent, PCI_COMMAND, &priv->bridge_pci_data.pci_command);
> > > +
> > > +     for (base = PCI_BASE_ADDRESS_0; base <= PCI_BASE_ADDRESS_5; base += 4, i++)
> > > +             pci_read_config_dword(parent, base, &priv->bridge_pci_data.base_address[i]);
> > > +
> > > +     pci_read_config_dword(parent, PCI_ROM_ADDRESS, &priv->bridge_pci_data.rom_addreess);
> > > +     pci_read_config_dword(parent, PCI_INTERRUPT_LINE, &priv->bridge_pci_data.interrupt_line);
> > > +
> > > +     pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_LO,
> > > +                           &priv->bridge_pci_data.msi_lo);
> > > +     pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_HI,
> > > +                           &priv->bridge_pci_data.msi_hi);
> > > +
> > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_DEVCTL,
> > > +                           &priv->bridge_pci_data.devctl);
> > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCAP,
> > > +                           &priv->bridge_pci_data.linkcap);
> > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCTL,
> > > +                           &priv->bridge_pci_data.linkctl_sts);
> > > +
> > > +     pci_read_config_dword(parent, LS7A_GEN2_CTL, &priv->bridge_pci_data.gen2_ctrl);
> > > +     priv->bridge_pci_data.gen2_ctrl |= FIELD_PREP(LS7A_GEN2_SPEED_CHANG, 0x1)
> > > +                                     | FIELD_PREP(LS7A_CONF_PHY_TX, 0x0);
> > > +
> > > +     pci_read_config_dword(parent, LS7A_SYMBOL_TIMER, &priv->bridge_pci_data.symbol_timer);
> > > +     priv->bridge_pci_data.symbol_timer |= LS7A_MASK_LEN_MATCH;
> > > +
> > > +     pci_read_config_dword(pdev, PCI_COMMAND, &priv->bmc_pci_data.pci_command);
> > > +     pci_read_config_dword(pdev, PCI_BASE_ADDRESS_0, &priv->bmc_pci_data.base_address0);
> > > +     pci_read_config_dword(pdev, PCI_INTERRUPT_LINE, &priv->bmc_pci_data.interrupt_line);
> > > +}
> > > +
> > > +static int ls2k_bmc_pdata_initial(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > > +{
> > > +     int gsi = 16 + (LS2K_BMC_RESET_GPIO & 7);
> > > +     void __iomem *gpio_base;
> > > +     int irq, ret;
> > > +
> > > +     ls2k_bmc_save_pci_data(pdev, priv);
> > > +
> > > +     INIT_WORK(&priv->bmc_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 PCI-E request_irq(%d) failed\n", pdev->irq);
> >
> > Please don't use function names in error messages.
> >
> > Make them human readable inclusive of non-kernel engineers.
> 
> How about:
> 
> dev_err(ddata->dev, "Failed to request LS2KBMC PCI-E IRQ %d.\n", pdev->irq);
> 
> also, the error message regarding GPIO IRQ is as follows:
> 
> dev_err(ddata->dev, "Failed to request LS2KBMC GPIO IRQ %d.\n", irq);

Yes, much better.

[...]

> > > +     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);
> >
> > priv (ddata) already contains dev - you don't need both.
> 
> Yes, we just pass priv(ddata), and
> 
> struct pci_dev *pdev = to_pci_dev(ddata->dev);

I would pass dev ... hold on, where do you store priv for reuse?

> > > +     if (ret)
> > > +             goto disable_pci;
> > > +
> > >       ret = ls2k_bmc_parse_mode(dev, &pd);
> > >       if (ret)
> > >               goto disable_pci;
> > > --
> > > 2.47.1
> > >
> >
> > --
> > Lee Jones [李琼斯]
> 
> --
> Thanks.
> Binbin

-- 
Lee Jones [李琼斯]

Re: [PATCH v7 1/3] mfd: ls2kbmc: Introduce Loongson-2K BMC core driver

[Lee Jones]

On Fri, 11 Jul 2025, Binbin Zhou wrote:

> Hi Lee:
> 
> Thanks for your review.
> 
> On Thu, Jul 10, 2025 at 5:56 PM Lee Jones <lee@kernel.org> wrote:
> >
> > On Fri, 04 Jul 2025, Binbin Zhou 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-3 CPU. It supports multiple sub-devices like DRM and IPMI.
> > >
> > > Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> > > Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> > > Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> > > Acked-by: Corey Minyard <corey@minyard.net>
> > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > ---
> > >  MAINTAINERS                 |   6 ++
> > >  drivers/mfd/Kconfig         |  13 +++
> > >  drivers/mfd/Makefile        |   2 +
> > >  drivers/mfd/ls2k-bmc-core.c | 156 ++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 177 insertions(+)

[...]

> > > +static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
> > > +{
> > > +     struct simplefb_platform_data pd;
> > > +     resource_size_t base;
> > > +     int ret;
> > > +
> > > +     ret = pci_enable_device(dev);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret = ls2k_bmc_parse_mode(dev, &pd);
> > > +     if (ret)
> > > +             goto disable_pci;
> > > +
> > > +     ls2k_bmc_cells[0].platform_data = &pd;
> > > +     ls2k_bmc_cells[0].pdata_size = sizeof(pd);
> >
> > This is fragile.
> >
> > Please identify the elements in ls2k_bmc_cells and use it to index here.
> >
> > See: `static struct mfd_cell as3711_subdevs`
> 
> How about this:
> 
> enum {
>         LS2K_BMC_DISPLAY,
>         LS2k_BMC_IPMI0,
>         LS2k_BMC_IPMI1,
>         LS2k_BMC_IPMI2,
>         LS2k_BMC_IPMI3,
>         LS2k_BMC_IPMI4,
> };
> 
> static struct mfd_cell ls2k_bmc_cells[] = {
>         [LS2K_BMC_DISPLAY] = {
>                 .name = "simple-framebuffer",
>                 .num_resources = ARRAY_SIZE(ls2k_display_resources),
>                 .resources = ls2k_display_resources
>         },
>         [LS2k_BMC_IPMI0] = {
>                 .name = "ls2k-ipmi-si",
>                 .num_resources = ARRAY_SIZE(ls2k_ipmi0_resources),
>                 .resources = ls2k_ipmi0_resources
>         },
>         [LS2k_BMC_IPMI1] = {
>                 .name = "ls2k-ipmi-si",
>                 .num_resources = ARRAY_SIZE(ls2k_ipmi1_resources),
>                 .resources = ls2k_ipmi1_resources
>         },
>         [LS2k_BMC_IPMI2] = {
>                 .name = "ls2k-ipmi-si",
>                 .num_resources = ARRAY_SIZE(ls2k_ipmi2_resources),
>                 .resources = ls2k_ipmi2_resources
>         },
>         [LS2k_BMC_IPMI3] = {
>                 .name = "ls2k-ipmi-si",
>                 .num_resources = ARRAY_SIZE(ls2k_ipmi3_resources),
>                 .resources = ls2k_ipmi3_resources
>         },
>         [LS2k_BMC_IPMI4] = {
>                 .name = "ls2k-ipmi-si",
>                 .num_resources = ARRAY_SIZE(ls2k_ipmi4_resources),
>                 .resources = ls2k_ipmi4_resources
>         },
> };
> 
> and
> 
>         ls2k_bmc_cells[LS2K_BMC_DISPLAY].platform_data = &pd;
>         ls2k_bmc_cells[LS2K_BMC_DISPLAY].pdata_size = sizeof(pd);

Yes, that's it.

-- 
Lee Jones [李琼斯]

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

[Binbin Zhou]

Hi Lee:

Thanks for your reply.


On Fri, Jul 18, 2025 at 9:52 PM Lee Jones <lee@kernel.org> wrote:
>
> On Fri, 11 Jul 2025, Binbin Zhou wrote:
>
> > Hi Lee:
> >
> > Thanks for your review.
> >
> > On Thu, Jul 10, 2025 at 6:03 PM Lee Jones <lee@kernel.org> wrote:
> > >
> > > On Fri, 04 Jul 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.
> > > >
> > > > Quick overview of the entire LS2K BMC reset process:
> > > >
> > > > There are two types of reset methods: soft reset (BMC-initiated reboot
> > > > of IPMI reset command) and BMC watchdog reset (watchdog timeout).
> > > >
> > > > First, regardless of the method, an interrupt is generated (PCIe interrupt
> > > > for soft reset/GPIO interrupt for watchdog reset);
> > > >
> > > > Second, during the interrupt process, the system enters bmc_reset_work,
> > > > clears the bus/IO/mem resources of the LS7A PCI-E bridge, waits for the BMC
> > > > reset to begin, then restores the parent device's PCI configuration space,
> > > > waits for the BMC reset to complete, and finally restores the BMC PCI
> > > > configuration space.
> > > >
> > > > Display restoration occurs last.
> > > >
> > > > Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> > > > Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> > > > Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> > > > Acked-by: Corey Minyard <corey@minyard.net>
> > > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > > ---
> > > >  drivers/mfd/ls2k-bmc-core.c | 328 ++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 328 insertions(+)
> > > >
> > > > diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> > > > index 50d560a4611c..1ae673f6a196 100644
> > > > --- a/drivers/mfd/ls2k-bmc-core.c
> > > > +++ b/drivers/mfd/ls2k-bmc-core.c
> > > > @@ -10,8 +10,12 @@
> > > >   */
> > > >
> > > >  #include <linux/aperture.h>
> > > > +#include <linux/bitfield.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>
> > > > @@ -19,6 +23,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>
> > > >
> > > >  /* LS2K BMC resources */
> > > >  #define LS2K_DISPLAY_RES_START               (SZ_16M + SZ_2M)
> > > > @@ -29,6 +35,48 @@
> > > >  #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 LS7A_PCI_CFG_SIZE            0x100
> > > > +
> > > > +/* LS7A bridge registers */
> > > > +#define LS7A_PCIE_PORT_CTL0          0x0
> > > > +#define LS7A_PCIE_PORT_STS1          0xC
> > > > +#define LS7A_GEN2_CTL                        0x80C
> > > > +#define LS7A_SYMBOL_TIMER            0x71C
> > > > +
> > > > +/* Bits of LS7A_PCIE_PORT_CTL0 */
> > > > +#define LS2K_BMC_PCIE_LTSSM_ENABLE   BIT(3)
> > > > +
> > > > +/* Bits of LS7A_PCIE_PORT_STS1 */
> > > > +#define LS2K_BMC_PCIE_LTSSM_STS              GENMASK(5, 0)
> > > > +#define LS2K_BMC_PCIE_CONNECTED              0x11
> > > > +
> > > > +#define LS2K_BMC_PCIE_DELAY_US               1000
> > > > +#define LS2K_BMC_PCIE_TIMEOUT_US     1000000
> > > > +
> > > > +/* Bits of LS7A_GEN2_CTL */
> > > > +#define LS7A_GEN2_SPEED_CHANG                BIT(17)
> > > > +#define LS7A_CONF_PHY_TX             BIT(18)
> > > > +
> > > > +/* Bits of LS7A_SYMBOL_TIMER */
> > > > +#define LS7A_MASK_LEN_MATCH          BIT(26)
> > > > +
> > > > +/* Interval between interruptions */
> > > > +#define LS2K_BMC_INT_INTERVAL                (60 * HZ)
> > > > +
> > > > +/* Maximum time to wait for U-Boot and DDR to be ready with ms. */
> > > > +#define LS2K_BMC_RESET_WAIT_TIME     10000
> > > > +
> > > > +/* It's an experience value */
> > > > +#define LS7A_BAR0_CHECK_MAX_TIMES    2000
> > > > +
> > > > +#define LS2K_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
> > > > +
> > > >  static struct resource ls2k_display_resources[] = {
> > > >       DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
> > > >  };
> > > > @@ -62,6 +110,273 @@ static struct mfd_cell ls2k_bmc_cells[] = {
> > > >       MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
> > > >  };
> > > >
> > > > +/* Index of the BMC PCI configuration space to be restored at BMC reset. */
> > > > +struct ls2k_bmc_pci_data {
> > > > +     u32 pci_command;
> > > > +     u32 base_address0;
> > > > +     u32 interrupt_line;
> > > > +};
> > > > +
> > > > +/* Index of the parent PCI configuration space to be restored at BMC reset. */
> > > > +struct ls2k_bmc_bridge_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;
> > > > +};
> > > > +
> > > > +struct ls2k_bmc_pdata {
> > > > +     struct device *dev;
> > > > +     struct work_struct bmc_reset_work;
> > > > +     struct ls2k_bmc_pci_data bmc_pci_data;
> > > > +     struct ls2k_bmc_bridge_pci_data bridge_pci_data;
> > > > +};
> > > > +
> > > > +static bool ls2k_bmc_bar0_addr_is_set(struct pci_dev *ppdev)
> > >
> > > Nit: This is usually called pdev.
> >
> > OK.
>
> Snip things you agree with please.
>
> [...]
>
> > > > +static void ls2k_bmc_events_fn(struct work_struct *work)
> > > > +{
> > > > +     struct ls2k_bmc_pdata *priv = container_of(work, struct ls2k_bmc_pdata, bmc_reset_work);
> > > > +
> > > > +     /*
> > > > +      * The PCI-E is lost when the BMC resets, at which point access to the PCI-E
> > > > +      * from other CPUs is suspended to prevent a crash.
> > > > +      */
> > > > +     stop_machine(ls2k_bmc_recover_pci_data, priv, NULL);
> > > > +
> > > > +#ifdef CONFIG_VT
> > >
> > > #ifery in C-files is generally frowned upon.
> > >
> > > Is the any pieces of run-time data you can use instead?
> > >
> > > Or a stub which culminated in a no-op if !CONFIG_VT?
> >
> > Emm, I'm not sure if I understand correctly, but is the following way suitable?
> >
> >         if (IS_ENABLED(CONFIG_VT))
>
> It's better than #if, but even better would be a stub in a header file.

Hmm... The declarations for vt_move_to_console()/set_console() are in
two VT-related header files [1]. Adding the relevant stub functions
involves the VT subsystem, which does not seem to be relevant to the
subject of our patch set.
Therefore, I think the above modification is more suitable for us.

[1]:
vt_move_to_console:
https://elixir.bootlin.com/linux/v6.15/source/include/linux/vt_kern.h#L141
set_console: https://elixir.bootlin.com/linux/v6.15/source/include/linux/kbd_kern.h#L69

>
> >                 /* Re-push the display due to previous PCI-E loss. */
> >                 set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> >
> > >
> > > > +     /* Re-push the display due to previous PCI-E loss. */
> > > > +     set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> > > > +#endif
> > > > +}
> > > > +
> > > > +static irqreturn_t ls2k_bmc_interrupt(int irq, void *arg)
> > > > +{
> > > > +     struct ls2k_bmc_pdata *priv = arg;
> > > > +     static unsigned long last_jiffies;
> > > > +
> > > > +     if (system_state != SYSTEM_RUNNING)
> > > > +             return IRQ_HANDLED;
> > > > +
> > > > +     /* Skip interrupt in LS2K_BMC_INT_INTERVAL */
> > > > +     if (time_after(jiffies, last_jiffies + LS2K_BMC_INT_INTERVAL)) {
> > > > +             schedule_work(&priv->bmc_reset_work);
> > > > +             last_jiffies = jiffies;
> > > > +     }
> > > > +
> > > > +     return IRQ_HANDLED;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Saves the BMC parent device (LS7A) and its own PCI configuration space registers
> > > > + * that need to be restored after BMC reset.
> > > > + */
> > > > +static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > > > +{
> > > > +     struct pci_dev *parent = pdev->bus->self;
> > > > +     int base, i = 0;
> > > > +
> > > > +     pci_read_config_dword(parent, PCI_COMMAND, &priv->bridge_pci_data.pci_command);
> > > > +
> > > > +     for (base = PCI_BASE_ADDRESS_0; base <= PCI_BASE_ADDRESS_5; base += 4, i++)
> > > > +             pci_read_config_dword(parent, base, &priv->bridge_pci_data.base_address[i]);
> > > > +
> > > > +     pci_read_config_dword(parent, PCI_ROM_ADDRESS, &priv->bridge_pci_data.rom_addreess);
> > > > +     pci_read_config_dword(parent, PCI_INTERRUPT_LINE, &priv->bridge_pci_data.interrupt_line);
> > > > +
> > > > +     pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_LO,
> > > > +                           &priv->bridge_pci_data.msi_lo);
> > > > +     pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_HI,
> > > > +                           &priv->bridge_pci_data.msi_hi);
> > > > +
> > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_DEVCTL,
> > > > +                           &priv->bridge_pci_data.devctl);
> > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCAP,
> > > > +                           &priv->bridge_pci_data.linkcap);
> > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCTL,
> > > > +                           &priv->bridge_pci_data.linkctl_sts);
> > > > +
> > > > +     pci_read_config_dword(parent, LS7A_GEN2_CTL, &priv->bridge_pci_data.gen2_ctrl);
> > > > +     priv->bridge_pci_data.gen2_ctrl |= FIELD_PREP(LS7A_GEN2_SPEED_CHANG, 0x1)
> > > > +                                     | FIELD_PREP(LS7A_CONF_PHY_TX, 0x0);
> > > > +
> > > > +     pci_read_config_dword(parent, LS7A_SYMBOL_TIMER, &priv->bridge_pci_data.symbol_timer);
> > > > +     priv->bridge_pci_data.symbol_timer |= LS7A_MASK_LEN_MATCH;
> > > > +
> > > > +     pci_read_config_dword(pdev, PCI_COMMAND, &priv->bmc_pci_data.pci_command);
> > > > +     pci_read_config_dword(pdev, PCI_BASE_ADDRESS_0, &priv->bmc_pci_data.base_address0);
> > > > +     pci_read_config_dword(pdev, PCI_INTERRUPT_LINE, &priv->bmc_pci_data.interrupt_line);
> > > > +}
> > > > +
> > > > +static int ls2k_bmc_pdata_initial(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > > > +{
> > > > +     int gsi = 16 + (LS2K_BMC_RESET_GPIO & 7);
> > > > +     void __iomem *gpio_base;
> > > > +     int irq, ret;
> > > > +
> > > > +     ls2k_bmc_save_pci_data(pdev, priv);
> > > > +
> > > > +     INIT_WORK(&priv->bmc_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 PCI-E request_irq(%d) failed\n", pdev->irq);
> > >
> > > Please don't use function names in error messages.
> > >
> > > Make them human readable inclusive of non-kernel engineers.
> >
> > How about:
> >
> > dev_err(ddata->dev, "Failed to request LS2KBMC PCI-E IRQ %d.\n", pdev->irq);
> >
> > also, the error message regarding GPIO IRQ is as follows:
> >
> > dev_err(ddata->dev, "Failed to request LS2KBMC GPIO IRQ %d.\n", irq);
>
> Yes, much better.
>
> [...]
>
> > > > +     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);
> > >
> > > priv (ddata) already contains dev - you don't need both.
> >
> > Yes, we just pass priv(ddata), and
> >
> > struct pci_dev *pdev = to_pci_dev(ddata->dev);
>
> I would pass dev ... hold on, where do you store priv for reuse?

Sorry, I am not entirely sure I understand your question. It does not
appear that priv needs to be reused.
>
> > > > +     if (ret)
> > > > +             goto disable_pci;
> > > > +
> > > >       ret = ls2k_bmc_parse_mode(dev, &pd);
> > > >       if (ret)
> > > >               goto disable_pci;
> > > > --
> > > > 2.47.1
> > > >
> > >
> > > --
> > > Lee Jones [李琼斯]
> >
> > --
> > Thanks.
> > Binbin
>
> --
> Lee Jones [李琼斯]

--
Thanks.
Binbin

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

[Lee Jones]

On Mon, 21 Jul 2025, Binbin Zhou wrote:

> Hi Lee:
> 
> Thanks for your reply.
> 
> 
> On Fri, Jul 18, 2025 at 9:52 PM Lee Jones <lee@kernel.org> wrote:
> >
> > On Fri, 11 Jul 2025, Binbin Zhou wrote:
> >
> > > Hi Lee:
> > >
> > > Thanks for your review.
> > >
> > > On Thu, Jul 10, 2025 at 6:03 PM Lee Jones <lee@kernel.org> wrote:
> > > >
> > > > On Fri, 04 Jul 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.
> > > > >
> > > > > Quick overview of the entire LS2K BMC reset process:
> > > > >
> > > > > There are two types of reset methods: soft reset (BMC-initiated reboot
> > > > > of IPMI reset command) and BMC watchdog reset (watchdog timeout).
> > > > >
> > > > > First, regardless of the method, an interrupt is generated (PCIe interrupt
> > > > > for soft reset/GPIO interrupt for watchdog reset);
> > > > >
> > > > > Second, during the interrupt process, the system enters bmc_reset_work,
> > > > > clears the bus/IO/mem resources of the LS7A PCI-E bridge, waits for the BMC
> > > > > reset to begin, then restores the parent device's PCI configuration space,
> > > > > waits for the BMC reset to complete, and finally restores the BMC PCI
> > > > > configuration space.
> > > > >
> > > > > Display restoration occurs last.
> > > > >
> > > > > Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> > > > > Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> > > > > Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> > > > > Acked-by: Corey Minyard <corey@minyard.net>
> > > > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > > > ---
> > > > >  drivers/mfd/ls2k-bmc-core.c | 328 ++++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 328 insertions(+)
> > > > >
> > > > > diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> > > > > index 50d560a4611c..1ae673f6a196 100644
> > > > > --- a/drivers/mfd/ls2k-bmc-core.c
> > > > > +++ b/drivers/mfd/ls2k-bmc-core.c
> > > > > @@ -10,8 +10,12 @@
> > > > >   */
> > > > >
> > > > >  #include <linux/aperture.h>
> > > > > +#include <linux/bitfield.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>
> > > > > @@ -19,6 +23,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>
> > > > >
> > > > >  /* LS2K BMC resources */
> > > > >  #define LS2K_DISPLAY_RES_START               (SZ_16M + SZ_2M)
> > > > > @@ -29,6 +35,48 @@
> > > > >  #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 LS7A_PCI_CFG_SIZE            0x100
> > > > > +
> > > > > +/* LS7A bridge registers */
> > > > > +#define LS7A_PCIE_PORT_CTL0          0x0
> > > > > +#define LS7A_PCIE_PORT_STS1          0xC
> > > > > +#define LS7A_GEN2_CTL                        0x80C
> > > > > +#define LS7A_SYMBOL_TIMER            0x71C
> > > > > +
> > > > > +/* Bits of LS7A_PCIE_PORT_CTL0 */
> > > > > +#define LS2K_BMC_PCIE_LTSSM_ENABLE   BIT(3)
> > > > > +
> > > > > +/* Bits of LS7A_PCIE_PORT_STS1 */
> > > > > +#define LS2K_BMC_PCIE_LTSSM_STS              GENMASK(5, 0)
> > > > > +#define LS2K_BMC_PCIE_CONNECTED              0x11
> > > > > +
> > > > > +#define LS2K_BMC_PCIE_DELAY_US               1000
> > > > > +#define LS2K_BMC_PCIE_TIMEOUT_US     1000000
> > > > > +
> > > > > +/* Bits of LS7A_GEN2_CTL */
> > > > > +#define LS7A_GEN2_SPEED_CHANG                BIT(17)
> > > > > +#define LS7A_CONF_PHY_TX             BIT(18)
> > > > > +
> > > > > +/* Bits of LS7A_SYMBOL_TIMER */
> > > > > +#define LS7A_MASK_LEN_MATCH          BIT(26)
> > > > > +
> > > > > +/* Interval between interruptions */
> > > > > +#define LS2K_BMC_INT_INTERVAL                (60 * HZ)
> > > > > +
> > > > > +/* Maximum time to wait for U-Boot and DDR to be ready with ms. */
> > > > > +#define LS2K_BMC_RESET_WAIT_TIME     10000
> > > > > +
> > > > > +/* It's an experience value */
> > > > > +#define LS7A_BAR0_CHECK_MAX_TIMES    2000
> > > > > +
> > > > > +#define LS2K_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
> > > > > +
> > > > >  static struct resource ls2k_display_resources[] = {
> > > > >       DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
> > > > >  };
> > > > > @@ -62,6 +110,273 @@ static struct mfd_cell ls2k_bmc_cells[] = {
> > > > >       MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
> > > > >  };
> > > > >
> > > > > +/* Index of the BMC PCI configuration space to be restored at BMC reset. */
> > > > > +struct ls2k_bmc_pci_data {
> > > > > +     u32 pci_command;
> > > > > +     u32 base_address0;
> > > > > +     u32 interrupt_line;
> > > > > +};
> > > > > +
> > > > > +/* Index of the parent PCI configuration space to be restored at BMC reset. */
> > > > > +struct ls2k_bmc_bridge_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;
> > > > > +};
> > > > > +
> > > > > +struct ls2k_bmc_pdata {
> > > > > +     struct device *dev;
> > > > > +     struct work_struct bmc_reset_work;
> > > > > +     struct ls2k_bmc_pci_data bmc_pci_data;
> > > > > +     struct ls2k_bmc_bridge_pci_data bridge_pci_data;
> > > > > +};
> > > > > +
> > > > > +static bool ls2k_bmc_bar0_addr_is_set(struct pci_dev *ppdev)
> > > >
> > > > Nit: This is usually called pdev.
> > >
> > > OK.
> >
> > Snip things you agree with please.
> >
> > [...]
> >
> > > > > +static void ls2k_bmc_events_fn(struct work_struct *work)
> > > > > +{
> > > > > +     struct ls2k_bmc_pdata *priv = container_of(work, struct ls2k_bmc_pdata, bmc_reset_work);
> > > > > +
> > > > > +     /*
> > > > > +      * The PCI-E is lost when the BMC resets, at which point access to the PCI-E
> > > > > +      * from other CPUs is suspended to prevent a crash.
> > > > > +      */
> > > > > +     stop_machine(ls2k_bmc_recover_pci_data, priv, NULL);
> > > > > +
> > > > > +#ifdef CONFIG_VT
> > > >
> > > > #ifery in C-files is generally frowned upon.
> > > >
> > > > Is the any pieces of run-time data you can use instead?
> > > >
> > > > Or a stub which culminated in a no-op if !CONFIG_VT?
> > >
> > > Emm, I'm not sure if I understand correctly, but is the following way suitable?
> > >
> > >         if (IS_ENABLED(CONFIG_VT))
> >
> > It's better than #if, but even better would be a stub in a header file.
> 
> Hmm... The declarations for vt_move_to_console()/set_console() are in
> two VT-related header files [1]. Adding the relevant stub functions
> involves the VT subsystem, which does not seem to be relevant to the
> subject of our patch set.
> Therefore, I think the above modification is more suitable for us.

All of the subsystems in the kernel are open source, last time I checked. :)

But okay, I won't insist on it.

> [1]:
> vt_move_to_console:
> https://elixir.bootlin.com/linux/v6.15/source/include/linux/vt_kern.h#L141
> set_console: https://elixir.bootlin.com/linux/v6.15/source/include/linux/kbd_kern.h#L69
> 
> >
> > >                 /* Re-push the display due to previous PCI-E loss. */
> > >                 set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> > >
> > > >
> > > > > +     /* Re-push the display due to previous PCI-E loss. */
> > > > > +     set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> > > > > +#endif
> > > > > +}
> > > > > +
> > > > > +static irqreturn_t ls2k_bmc_interrupt(int irq, void *arg)
> > > > > +{
> > > > > +     struct ls2k_bmc_pdata *priv = arg;
> > > > > +     static unsigned long last_jiffies;
> > > > > +
> > > > > +     if (system_state != SYSTEM_RUNNING)
> > > > > +             return IRQ_HANDLED;
> > > > > +
> > > > > +     /* Skip interrupt in LS2K_BMC_INT_INTERVAL */
> > > > > +     if (time_after(jiffies, last_jiffies + LS2K_BMC_INT_INTERVAL)) {
> > > > > +             schedule_work(&priv->bmc_reset_work);
> > > > > +             last_jiffies = jiffies;
> > > > > +     }
> > > > > +
> > > > > +     return IRQ_HANDLED;
> > > > > +}
> > > > > +
> > > > > +/*
> > > > > + * Saves the BMC parent device (LS7A) and its own PCI configuration space registers
> > > > > + * that need to be restored after BMC reset.
> > > > > + */
> > > > > +static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > > > > +{
> > > > > +     struct pci_dev *parent = pdev->bus->self;
> > > > > +     int base, i = 0;
> > > > > +
> > > > > +     pci_read_config_dword(parent, PCI_COMMAND, &priv->bridge_pci_data.pci_command);
> > > > > +
> > > > > +     for (base = PCI_BASE_ADDRESS_0; base <= PCI_BASE_ADDRESS_5; base += 4, i++)
> > > > > +             pci_read_config_dword(parent, base, &priv->bridge_pci_data.base_address[i]);
> > > > > +
> > > > > +     pci_read_config_dword(parent, PCI_ROM_ADDRESS, &priv->bridge_pci_data.rom_addreess);
> > > > > +     pci_read_config_dword(parent, PCI_INTERRUPT_LINE, &priv->bridge_pci_data.interrupt_line);
> > > > > +
> > > > > +     pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_LO,
> > > > > +                           &priv->bridge_pci_data.msi_lo);
> > > > > +     pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_HI,
> > > > > +                           &priv->bridge_pci_data.msi_hi);
> > > > > +
> > > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_DEVCTL,
> > > > > +                           &priv->bridge_pci_data.devctl);
> > > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCAP,
> > > > > +                           &priv->bridge_pci_data.linkcap);
> > > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCTL,
> > > > > +                           &priv->bridge_pci_data.linkctl_sts);
> > > > > +
> > > > > +     pci_read_config_dword(parent, LS7A_GEN2_CTL, &priv->bridge_pci_data.gen2_ctrl);
> > > > > +     priv->bridge_pci_data.gen2_ctrl |= FIELD_PREP(LS7A_GEN2_SPEED_CHANG, 0x1)
> > > > > +                                     | FIELD_PREP(LS7A_CONF_PHY_TX, 0x0);
> > > > > +
> > > > > +     pci_read_config_dword(parent, LS7A_SYMBOL_TIMER, &priv->bridge_pci_data.symbol_timer);
> > > > > +     priv->bridge_pci_data.symbol_timer |= LS7A_MASK_LEN_MATCH;
> > > > > +
> > > > > +     pci_read_config_dword(pdev, PCI_COMMAND, &priv->bmc_pci_data.pci_command);
> > > > > +     pci_read_config_dword(pdev, PCI_BASE_ADDRESS_0, &priv->bmc_pci_data.base_address0);
> > > > > +     pci_read_config_dword(pdev, PCI_INTERRUPT_LINE, &priv->bmc_pci_data.interrupt_line);
> > > > > +}
> > > > > +
> > > > > +static int ls2k_bmc_pdata_initial(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > > > > +{
> > > > > +     int gsi = 16 + (LS2K_BMC_RESET_GPIO & 7);
> > > > > +     void __iomem *gpio_base;
> > > > > +     int irq, ret;
> > > > > +
> > > > > +     ls2k_bmc_save_pci_data(pdev, priv);
> > > > > +
> > > > > +     INIT_WORK(&priv->bmc_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 PCI-E request_irq(%d) failed\n", pdev->irq);
> > > >
> > > > Please don't use function names in error messages.
> > > >
> > > > Make them human readable inclusive of non-kernel engineers.
> > >
> > > How about:
> > >
> > > dev_err(ddata->dev, "Failed to request LS2KBMC PCI-E IRQ %d.\n", pdev->irq);
> > >
> > > also, the error message regarding GPIO IRQ is as follows:
> > >
> > > dev_err(ddata->dev, "Failed to request LS2KBMC GPIO IRQ %d.\n", irq);
> >
> > Yes, much better.
> >
> > [...]
> >
> > > > > +     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);
> > > >
> > > > priv (ddata) already contains dev - you don't need both.
> > >
> > > Yes, we just pass priv(ddata), and
> > >
> > > struct pci_dev *pdev = to_pci_dev(ddata->dev);
> >
> > I would pass dev ... hold on, where do you store priv for reuse?
> 
> Sorry, I am not entirely sure I understand your question. It does not
> appear that priv needs to be reused.

Then why does it exist at all?

-- 
Lee Jones [李琼斯]

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

[Binbin Zhou]

Hi Lee:

Thanks for your reply again.

On Wed, Jul 23, 2025 at 3:31 PM Lee Jones <lee@kernel.org> wrote:
>
> On Mon, 21 Jul 2025, Binbin Zhou wrote:
>
> > Hi Lee:
> >
> > Thanks for your reply.
> >
> >
> > On Fri, Jul 18, 2025 at 9:52 PM Lee Jones <lee@kernel.org> wrote:
> > >
> > > On Fri, 11 Jul 2025, Binbin Zhou wrote:
> > >
> > > > Hi Lee:
> > > >
> > > > Thanks for your review.
> > > >
> > > > On Thu, Jul 10, 2025 at 6:03 PM Lee Jones <lee@kernel.org> wrote:
> > > > >
> > > > > On Fri, 04 Jul 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.
> > > > > >
> > > > > > Quick overview of the entire LS2K BMC reset process:
> > > > > >
> > > > > > There are two types of reset methods: soft reset (BMC-initiated reboot
> > > > > > of IPMI reset command) and BMC watchdog reset (watchdog timeout).
> > > > > >
> > > > > > First, regardless of the method, an interrupt is generated (PCIe interrupt
> > > > > > for soft reset/GPIO interrupt for watchdog reset);
> > > > > >
> > > > > > Second, during the interrupt process, the system enters bmc_reset_work,
> > > > > > clears the bus/IO/mem resources of the LS7A PCI-E bridge, waits for the BMC
> > > > > > reset to begin, then restores the parent device's PCI configuration space,
> > > > > > waits for the BMC reset to complete, and finally restores the BMC PCI
> > > > > > configuration space.
> > > > > >
> > > > > > Display restoration occurs last.
> > > > > >
> > > > > > Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> > > > > > Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> > > > > > Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> > > > > > Acked-by: Corey Minyard <corey@minyard.net>
> > > > > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > > > > ---
> > > > > >  drivers/mfd/ls2k-bmc-core.c | 328 ++++++++++++++++++++++++++++++++++++
> > > > > >  1 file changed, 328 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> > > > > > index 50d560a4611c..1ae673f6a196 100644
> > > > > > --- a/drivers/mfd/ls2k-bmc-core.c
> > > > > > +++ b/drivers/mfd/ls2k-bmc-core.c
> > > > > > @@ -10,8 +10,12 @@
> > > > > >   */
> > > > > >
> > > > > >  #include <linux/aperture.h>
> > > > > > +#include <linux/bitfield.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>
> > > > > > @@ -19,6 +23,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>
> > > > > >
> > > > > >  /* LS2K BMC resources */
> > > > > >  #define LS2K_DISPLAY_RES_START               (SZ_16M + SZ_2M)
> > > > > > @@ -29,6 +35,48 @@
> > > > > >  #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 LS7A_PCI_CFG_SIZE            0x100
> > > > > > +
> > > > > > +/* LS7A bridge registers */
> > > > > > +#define LS7A_PCIE_PORT_CTL0          0x0
> > > > > > +#define LS7A_PCIE_PORT_STS1          0xC
> > > > > > +#define LS7A_GEN2_CTL                        0x80C
> > > > > > +#define LS7A_SYMBOL_TIMER            0x71C
> > > > > > +
> > > > > > +/* Bits of LS7A_PCIE_PORT_CTL0 */
> > > > > > +#define LS2K_BMC_PCIE_LTSSM_ENABLE   BIT(3)
> > > > > > +
> > > > > > +/* Bits of LS7A_PCIE_PORT_STS1 */
> > > > > > +#define LS2K_BMC_PCIE_LTSSM_STS              GENMASK(5, 0)
> > > > > > +#define LS2K_BMC_PCIE_CONNECTED              0x11
> > > > > > +
> > > > > > +#define LS2K_BMC_PCIE_DELAY_US               1000
> > > > > > +#define LS2K_BMC_PCIE_TIMEOUT_US     1000000
> > > > > > +
> > > > > > +/* Bits of LS7A_GEN2_CTL */
> > > > > > +#define LS7A_GEN2_SPEED_CHANG                BIT(17)
> > > > > > +#define LS7A_CONF_PHY_TX             BIT(18)
> > > > > > +
> > > > > > +/* Bits of LS7A_SYMBOL_TIMER */
> > > > > > +#define LS7A_MASK_LEN_MATCH          BIT(26)
> > > > > > +
> > > > > > +/* Interval between interruptions */
> > > > > > +#define LS2K_BMC_INT_INTERVAL                (60 * HZ)
> > > > > > +
> > > > > > +/* Maximum time to wait for U-Boot and DDR to be ready with ms. */
> > > > > > +#define LS2K_BMC_RESET_WAIT_TIME     10000
> > > > > > +
> > > > > > +/* It's an experience value */
> > > > > > +#define LS7A_BAR0_CHECK_MAX_TIMES    2000
> > > > > > +
> > > > > > +#define LS2K_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
> > > > > > +
> > > > > >  static struct resource ls2k_display_resources[] = {
> > > > > >       DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
> > > > > >  };
> > > > > > @@ -62,6 +110,273 @@ static struct mfd_cell ls2k_bmc_cells[] = {
> > > > > >       MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
> > > > > >  };
> > > > > >
> > > > > > +/* Index of the BMC PCI configuration space to be restored at BMC reset. */
> > > > > > +struct ls2k_bmc_pci_data {
> > > > > > +     u32 pci_command;
> > > > > > +     u32 base_address0;
> > > > > > +     u32 interrupt_line;
> > > > > > +};
> > > > > > +
> > > > > > +/* Index of the parent PCI configuration space to be restored at BMC reset. */
> > > > > > +struct ls2k_bmc_bridge_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;
> > > > > > +};
> > > > > > +
> > > > > > +struct ls2k_bmc_pdata {
> > > > > > +     struct device *dev;
> > > > > > +     struct work_struct bmc_reset_work;
> > > > > > +     struct ls2k_bmc_pci_data bmc_pci_data;
> > > > > > +     struct ls2k_bmc_bridge_pci_data bridge_pci_data;
> > > > > > +};
> > > > > > +
> > > > > > +static bool ls2k_bmc_bar0_addr_is_set(struct pci_dev *ppdev)
> > > > >
> > > > > Nit: This is usually called pdev.
> > > >
> > > > OK.
> > >
> > > Snip things you agree with please.
> > >
> > > [...]
> > >
> > > > > > +static void ls2k_bmc_events_fn(struct work_struct *work)
> > > > > > +{
> > > > > > +     struct ls2k_bmc_pdata *priv = container_of(work, struct ls2k_bmc_pdata, bmc_reset_work);
> > > > > > +
> > > > > > +     /*
> > > > > > +      * The PCI-E is lost when the BMC resets, at which point access to the PCI-E
> > > > > > +      * from other CPUs is suspended to prevent a crash.
> > > > > > +      */
> > > > > > +     stop_machine(ls2k_bmc_recover_pci_data, priv, NULL);
> > > > > > +
> > > > > > +#ifdef CONFIG_VT
> > > > >
> > > > > #ifery in C-files is generally frowned upon.
> > > > >
> > > > > Is the any pieces of run-time data you can use instead?
> > > > >
> > > > > Or a stub which culminated in a no-op if !CONFIG_VT?
> > > >
> > > > Emm, I'm not sure if I understand correctly, but is the following way suitable?
> > > >
> > > >         if (IS_ENABLED(CONFIG_VT))
> > >
> > > It's better than #if, but even better would be a stub in a header file.
> >
> > Hmm... The declarations for vt_move_to_console()/set_console() are in
> > two VT-related header files [1]. Adding the relevant stub functions
> > involves the VT subsystem, which does not seem to be relevant to the
> > subject of our patch set.
> > Therefore, I think the above modification is more suitable for us.
>
> All of the subsystems in the kernel are open source, last time I checked. :)
>
> But okay, I won't insist on it.
>
> > [1]:
> > vt_move_to_console:
> > https://elixir.bootlin.com/linux/v6.15/source/include/linux/vt_kern.h#L141
> > set_console: https://elixir.bootlin.com/linux/v6.15/source/include/linux/kbd_kern.h#L69
> >
> > >
> > > >                 /* Re-push the display due to previous PCI-E loss. */
> > > >                 set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> > > >
> > > > >
> > > > > > +     /* Re-push the display due to previous PCI-E loss. */
> > > > > > +     set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> > > > > > +#endif
> > > > > > +}
> > > > > > +
> > > > > > +static irqreturn_t ls2k_bmc_interrupt(int irq, void *arg)
> > > > > > +{
> > > > > > +     struct ls2k_bmc_pdata *priv = arg;
> > > > > > +     static unsigned long last_jiffies;
> > > > > > +
> > > > > > +     if (system_state != SYSTEM_RUNNING)
> > > > > > +             return IRQ_HANDLED;
> > > > > > +
> > > > > > +     /* Skip interrupt in LS2K_BMC_INT_INTERVAL */
> > > > > > +     if (time_after(jiffies, last_jiffies + LS2K_BMC_INT_INTERVAL)) {
> > > > > > +             schedule_work(&priv->bmc_reset_work);
> > > > > > +             last_jiffies = jiffies;
> > > > > > +     }
> > > > > > +
> > > > > > +     return IRQ_HANDLED;
> > > > > > +}
> > > > > > +
> > > > > > +/*
> > > > > > + * Saves the BMC parent device (LS7A) and its own PCI configuration space registers
> > > > > > + * that need to be restored after BMC reset.
> > > > > > + */
> > > > > > +static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > > > > > +{
> > > > > > +     struct pci_dev *parent = pdev->bus->self;
> > > > > > +     int base, i = 0;
> > > > > > +
> > > > > > +     pci_read_config_dword(parent, PCI_COMMAND, &priv->bridge_pci_data.pci_command);
> > > > > > +
> > > > > > +     for (base = PCI_BASE_ADDRESS_0; base <= PCI_BASE_ADDRESS_5; base += 4, i++)
> > > > > > +             pci_read_config_dword(parent, base, &priv->bridge_pci_data.base_address[i]);
> > > > > > +
> > > > > > +     pci_read_config_dword(parent, PCI_ROM_ADDRESS, &priv->bridge_pci_data.rom_addreess);
> > > > > > +     pci_read_config_dword(parent, PCI_INTERRUPT_LINE, &priv->bridge_pci_data.interrupt_line);
> > > > > > +
> > > > > > +     pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_LO,
> > > > > > +                           &priv->bridge_pci_data.msi_lo);
> > > > > > +     pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_HI,
> > > > > > +                           &priv->bridge_pci_data.msi_hi);
> > > > > > +
> > > > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_DEVCTL,
> > > > > > +                           &priv->bridge_pci_data.devctl);
> > > > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCAP,
> > > > > > +                           &priv->bridge_pci_data.linkcap);
> > > > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCTL,
> > > > > > +                           &priv->bridge_pci_data.linkctl_sts);
> > > > > > +
> > > > > > +     pci_read_config_dword(parent, LS7A_GEN2_CTL, &priv->bridge_pci_data.gen2_ctrl);
> > > > > > +     priv->bridge_pci_data.gen2_ctrl |= FIELD_PREP(LS7A_GEN2_SPEED_CHANG, 0x1)
> > > > > > +                                     | FIELD_PREP(LS7A_CONF_PHY_TX, 0x0);
> > > > > > +
> > > > > > +     pci_read_config_dword(parent, LS7A_SYMBOL_TIMER, &priv->bridge_pci_data.symbol_timer);
> > > > > > +     priv->bridge_pci_data.symbol_timer |= LS7A_MASK_LEN_MATCH;
> > > > > > +
> > > > > > +     pci_read_config_dword(pdev, PCI_COMMAND, &priv->bmc_pci_data.pci_command);
> > > > > > +     pci_read_config_dword(pdev, PCI_BASE_ADDRESS_0, &priv->bmc_pci_data.base_address0);
> > > > > > +     pci_read_config_dword(pdev, PCI_INTERRUPT_LINE, &priv->bmc_pci_data.interrupt_line);
> > > > > > +}
> > > > > > +
> > > > > > +static int ls2k_bmc_pdata_initial(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > > > > > +{
> > > > > > +     int gsi = 16 + (LS2K_BMC_RESET_GPIO & 7);
> > > > > > +     void __iomem *gpio_base;
> > > > > > +     int irq, ret;
> > > > > > +
> > > > > > +     ls2k_bmc_save_pci_data(pdev, priv);
> > > > > > +
> > > > > > +     INIT_WORK(&priv->bmc_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 PCI-E request_irq(%d) failed\n", pdev->irq);
> > > > >
> > > > > Please don't use function names in error messages.
> > > > >
> > > > > Make them human readable inclusive of non-kernel engineers.
> > > >
> > > > How about:
> > > >
> > > > dev_err(ddata->dev, "Failed to request LS2KBMC PCI-E IRQ %d.\n", pdev->irq);
> > > >
> > > > also, the error message regarding GPIO IRQ is as follows:
> > > >
> > > > dev_err(ddata->dev, "Failed to request LS2KBMC GPIO IRQ %d.\n", irq);
> > >
> > > Yes, much better.
> > >
> > > [...]
> > >
> > > > > > +     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);
> > > > >
> > > > > priv (ddata) already contains dev - you don't need both.
> > > >
> > > > Yes, we just pass priv(ddata), and
> > > >
> > > > struct pci_dev *pdev = to_pci_dev(ddata->dev);
> > >
> > > I would pass dev ... hold on, where do you store priv for reuse?
> >
> > Sorry, I am not entirely sure I understand your question. It does not
> > appear that priv needs to be reused.
>
> Then why does it exist at all?

Emm...
Perhaps I have strayed from the topic of our discussion. Let's try to
get back on track.
After replying to your previous email, I thought about whether your
question was whether we need something similar to

pci_set_drvdata(dev, priv);

Looking at the code as a whole, perhaps it is because I am used to
using `priv` as a function parameter, so in most cases I use `priv`
directly, except in interrupt routines and INIT_WORK
I hope we are on the right track with our discussion.
>
> --
> Lee Jones [李琼斯]

-- 
Thanks.
Binbin

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

[Lee Jones]

On Wed, 23 Jul 2025, Binbin Zhou wrote:

> Hi Lee:
> 
> Thanks for your reply again.
> 
> On Wed, Jul 23, 2025 at 3:31 PM Lee Jones <lee@kernel.org> wrote:
> >
> > On Mon, 21 Jul 2025, Binbin Zhou wrote:
> >
> > > Hi Lee:
> > >
> > > Thanks for your reply.
> > >
> > >
> > > On Fri, Jul 18, 2025 at 9:52 PM Lee Jones <lee@kernel.org> wrote:
> > > >
> > > > On Fri, 11 Jul 2025, Binbin Zhou wrote:
> > > >
> > > > > Hi Lee:
> > > > >
> > > > > Thanks for your review.
> > > > >
> > > > > On Thu, Jul 10, 2025 at 6:03 PM Lee Jones <lee@kernel.org> wrote:
> > > > > >
> > > > > > On Fri, 04 Jul 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.
> > > > > > >
> > > > > > > Quick overview of the entire LS2K BMC reset process:
> > > > > > >
> > > > > > > There are two types of reset methods: soft reset (BMC-initiated reboot
> > > > > > > of IPMI reset command) and BMC watchdog reset (watchdog timeout).
> > > > > > >
> > > > > > > First, regardless of the method, an interrupt is generated (PCIe interrupt
> > > > > > > for soft reset/GPIO interrupt for watchdog reset);
> > > > > > >
> > > > > > > Second, during the interrupt process, the system enters bmc_reset_work,
> > > > > > > clears the bus/IO/mem resources of the LS7A PCI-E bridge, waits for the BMC
> > > > > > > reset to begin, then restores the parent device's PCI configuration space,
> > > > > > > waits for the BMC reset to complete, and finally restores the BMC PCI
> > > > > > > configuration space.
> > > > > > >
> > > > > > > Display restoration occurs last.
> > > > > > >
> > > > > > > Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> > > > > > > Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> > > > > > > Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> > > > > > > Acked-by: Corey Minyard <corey@minyard.net>
> > > > > > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > > > > > ---
> > > > > > >  drivers/mfd/ls2k-bmc-core.c | 328 ++++++++++++++++++++++++++++++++++++
> > > > > > >  1 file changed, 328 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> > > > > > > index 50d560a4611c..1ae673f6a196 100644
> > > > > > > --- a/drivers/mfd/ls2k-bmc-core.c
> > > > > > > +++ b/drivers/mfd/ls2k-bmc-core.c
> > > > > > > @@ -10,8 +10,12 @@
> > > > > > >   */
> > > > > > >
> > > > > > >  #include <linux/aperture.h>
> > > > > > > +#include <linux/bitfield.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>
> > > > > > > @@ -19,6 +23,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>
> > > > > > >
> > > > > > >  /* LS2K BMC resources */
> > > > > > >  #define LS2K_DISPLAY_RES_START               (SZ_16M + SZ_2M)
> > > > > > > @@ -29,6 +35,48 @@
> > > > > > >  #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 LS7A_PCI_CFG_SIZE            0x100
> > > > > > > +
> > > > > > > +/* LS7A bridge registers */
> > > > > > > +#define LS7A_PCIE_PORT_CTL0          0x0
> > > > > > > +#define LS7A_PCIE_PORT_STS1          0xC
> > > > > > > +#define LS7A_GEN2_CTL                        0x80C
> > > > > > > +#define LS7A_SYMBOL_TIMER            0x71C
> > > > > > > +
> > > > > > > +/* Bits of LS7A_PCIE_PORT_CTL0 */
> > > > > > > +#define LS2K_BMC_PCIE_LTSSM_ENABLE   BIT(3)
> > > > > > > +
> > > > > > > +/* Bits of LS7A_PCIE_PORT_STS1 */
> > > > > > > +#define LS2K_BMC_PCIE_LTSSM_STS              GENMASK(5, 0)
> > > > > > > +#define LS2K_BMC_PCIE_CONNECTED              0x11
> > > > > > > +
> > > > > > > +#define LS2K_BMC_PCIE_DELAY_US               1000
> > > > > > > +#define LS2K_BMC_PCIE_TIMEOUT_US     1000000
> > > > > > > +
> > > > > > > +/* Bits of LS7A_GEN2_CTL */
> > > > > > > +#define LS7A_GEN2_SPEED_CHANG                BIT(17)
> > > > > > > +#define LS7A_CONF_PHY_TX             BIT(18)
> > > > > > > +
> > > > > > > +/* Bits of LS7A_SYMBOL_TIMER */
> > > > > > > +#define LS7A_MASK_LEN_MATCH          BIT(26)
> > > > > > > +
> > > > > > > +/* Interval between interruptions */
> > > > > > > +#define LS2K_BMC_INT_INTERVAL                (60 * HZ)
> > > > > > > +
> > > > > > > +/* Maximum time to wait for U-Boot and DDR to be ready with ms. */
> > > > > > > +#define LS2K_BMC_RESET_WAIT_TIME     10000
> > > > > > > +
> > > > > > > +/* It's an experience value */
> > > > > > > +#define LS7A_BAR0_CHECK_MAX_TIMES    2000
> > > > > > > +
> > > > > > > +#define LS2K_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
> > > > > > > +
> > > > > > >  static struct resource ls2k_display_resources[] = {
> > > > > > >       DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
> > > > > > >  };
> > > > > > > @@ -62,6 +110,273 @@ static struct mfd_cell ls2k_bmc_cells[] = {
> > > > > > >       MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
> > > > > > >  };
> > > > > > >
> > > > > > > +/* Index of the BMC PCI configuration space to be restored at BMC reset. */
> > > > > > > +struct ls2k_bmc_pci_data {
> > > > > > > +     u32 pci_command;
> > > > > > > +     u32 base_address0;
> > > > > > > +     u32 interrupt_line;
> > > > > > > +};
> > > > > > > +
> > > > > > > +/* Index of the parent PCI configuration space to be restored at BMC reset. */
> > > > > > > +struct ls2k_bmc_bridge_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;
> > > > > > > +};
> > > > > > > +
> > > > > > > +struct ls2k_bmc_pdata {
> > > > > > > +     struct device *dev;
> > > > > > > +     struct work_struct bmc_reset_work;
> > > > > > > +     struct ls2k_bmc_pci_data bmc_pci_data;
> > > > > > > +     struct ls2k_bmc_bridge_pci_data bridge_pci_data;
> > > > > > > +};
> > > > > > > +
> > > > > > > +static bool ls2k_bmc_bar0_addr_is_set(struct pci_dev *ppdev)
> > > > > >
> > > > > > Nit: This is usually called pdev.
> > > > >
> > > > > OK.
> > > >
> > > > Snip things you agree with please.
> > > >
> > > > [...]
> > > >
> > > > > > > +static void ls2k_bmc_events_fn(struct work_struct *work)
> > > > > > > +{
> > > > > > > +     struct ls2k_bmc_pdata *priv = container_of(work, struct ls2k_bmc_pdata, bmc_reset_work);
> > > > > > > +
> > > > > > > +     /*
> > > > > > > +      * The PCI-E is lost when the BMC resets, at which point access to the PCI-E
> > > > > > > +      * from other CPUs is suspended to prevent a crash.
> > > > > > > +      */
> > > > > > > +     stop_machine(ls2k_bmc_recover_pci_data, priv, NULL);
> > > > > > > +
> > > > > > > +#ifdef CONFIG_VT
> > > > > >
> > > > > > #ifery in C-files is generally frowned upon.
> > > > > >
> > > > > > Is the any pieces of run-time data you can use instead?
> > > > > >
> > > > > > Or a stub which culminated in a no-op if !CONFIG_VT?
> > > > >
> > > > > Emm, I'm not sure if I understand correctly, but is the following way suitable?
> > > > >
> > > > >         if (IS_ENABLED(CONFIG_VT))
> > > >
> > > > It's better than #if, but even better would be a stub in a header file.
> > >
> > > Hmm... The declarations for vt_move_to_console()/set_console() are in
> > > two VT-related header files [1]. Adding the relevant stub functions
> > > involves the VT subsystem, which does not seem to be relevant to the
> > > subject of our patch set.
> > > Therefore, I think the above modification is more suitable for us.
> >
> > All of the subsystems in the kernel are open source, last time I checked. :)
> >
> > But okay, I won't insist on it.
> >
> > > [1]:
> > > vt_move_to_console:
> > > https://elixir.bootlin.com/linux/v6.15/source/include/linux/vt_kern.h#L141
> > > set_console: https://elixir.bootlin.com/linux/v6.15/source/include/linux/kbd_kern.h#L69
> > >
> > > >
> > > > >                 /* Re-push the display due to previous PCI-E loss. */
> > > > >                 set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> > > > >
> > > > > >
> > > > > > > +     /* Re-push the display due to previous PCI-E loss. */
> > > > > > > +     set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> > > > > > > +#endif
> > > > > > > +}
> > > > > > > +
> > > > > > > +static irqreturn_t ls2k_bmc_interrupt(int irq, void *arg)
> > > > > > > +{
> > > > > > > +     struct ls2k_bmc_pdata *priv = arg;
> > > > > > > +     static unsigned long last_jiffies;
> > > > > > > +
> > > > > > > +     if (system_state != SYSTEM_RUNNING)
> > > > > > > +             return IRQ_HANDLED;
> > > > > > > +
> > > > > > > +     /* Skip interrupt in LS2K_BMC_INT_INTERVAL */
> > > > > > > +     if (time_after(jiffies, last_jiffies + LS2K_BMC_INT_INTERVAL)) {
> > > > > > > +             schedule_work(&priv->bmc_reset_work);
> > > > > > > +             last_jiffies = jiffies;
> > > > > > > +     }
> > > > > > > +
> > > > > > > +     return IRQ_HANDLED;
> > > > > > > +}
> > > > > > > +
> > > > > > > +/*
> > > > > > > + * Saves the BMC parent device (LS7A) and its own PCI configuration space registers
> > > > > > > + * that need to be restored after BMC reset.
> > > > > > > + */
> > > > > > > +static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > > > > > > +{
> > > > > > > +     struct pci_dev *parent = pdev->bus->self;
> > > > > > > +     int base, i = 0;
> > > > > > > +
> > > > > > > +     pci_read_config_dword(parent, PCI_COMMAND, &priv->bridge_pci_data.pci_command);
> > > > > > > +
> > > > > > > +     for (base = PCI_BASE_ADDRESS_0; base <= PCI_BASE_ADDRESS_5; base += 4, i++)
> > > > > > > +             pci_read_config_dword(parent, base, &priv->bridge_pci_data.base_address[i]);
> > > > > > > +
> > > > > > > +     pci_read_config_dword(parent, PCI_ROM_ADDRESS, &priv->bridge_pci_data.rom_addreess);
> > > > > > > +     pci_read_config_dword(parent, PCI_INTERRUPT_LINE, &priv->bridge_pci_data.interrupt_line);
> > > > > > > +
> > > > > > > +     pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_LO,
> > > > > > > +                           &priv->bridge_pci_data.msi_lo);
> > > > > > > +     pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_HI,
> > > > > > > +                           &priv->bridge_pci_data.msi_hi);
> > > > > > > +
> > > > > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_DEVCTL,
> > > > > > > +                           &priv->bridge_pci_data.devctl);
> > > > > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCAP,
> > > > > > > +                           &priv->bridge_pci_data.linkcap);
> > > > > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCTL,
> > > > > > > +                           &priv->bridge_pci_data.linkctl_sts);
> > > > > > > +
> > > > > > > +     pci_read_config_dword(parent, LS7A_GEN2_CTL, &priv->bridge_pci_data.gen2_ctrl);
> > > > > > > +     priv->bridge_pci_data.gen2_ctrl |= FIELD_PREP(LS7A_GEN2_SPEED_CHANG, 0x1)
> > > > > > > +                                     | FIELD_PREP(LS7A_CONF_PHY_TX, 0x0);
> > > > > > > +
> > > > > > > +     pci_read_config_dword(parent, LS7A_SYMBOL_TIMER, &priv->bridge_pci_data.symbol_timer);
> > > > > > > +     priv->bridge_pci_data.symbol_timer |= LS7A_MASK_LEN_MATCH;
> > > > > > > +
> > > > > > > +     pci_read_config_dword(pdev, PCI_COMMAND, &priv->bmc_pci_data.pci_command);
> > > > > > > +     pci_read_config_dword(pdev, PCI_BASE_ADDRESS_0, &priv->bmc_pci_data.base_address0);
> > > > > > > +     pci_read_config_dword(pdev, PCI_INTERRUPT_LINE, &priv->bmc_pci_data.interrupt_line);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static int ls2k_bmc_pdata_initial(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > > > > > > +{
> > > > > > > +     int gsi = 16 + (LS2K_BMC_RESET_GPIO & 7);
> > > > > > > +     void __iomem *gpio_base;
> > > > > > > +     int irq, ret;
> > > > > > > +
> > > > > > > +     ls2k_bmc_save_pci_data(pdev, priv);
> > > > > > > +
> > > > > > > +     INIT_WORK(&priv->bmc_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 PCI-E request_irq(%d) failed\n", pdev->irq);
> > > > > >
> > > > > > Please don't use function names in error messages.
> > > > > >
> > > > > > Make them human readable inclusive of non-kernel engineers.
> > > > >
> > > > > How about:
> > > > >
> > > > > dev_err(ddata->dev, "Failed to request LS2KBMC PCI-E IRQ %d.\n", pdev->irq);
> > > > >
> > > > > also, the error message regarding GPIO IRQ is as follows:
> > > > >
> > > > > dev_err(ddata->dev, "Failed to request LS2KBMC GPIO IRQ %d.\n", irq);
> > > >
> > > > Yes, much better.
> > > >
> > > > [...]
> > > >
> > > > > > > +     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);
> > > > > >
> > > > > > priv (ddata) already contains dev - you don't need both.
> > > > >
> > > > > Yes, we just pass priv(ddata), and
> > > > >
> > > > > struct pci_dev *pdev = to_pci_dev(ddata->dev);
> > > >
> > > > I would pass dev ... hold on, where do you store priv for reuse?
> > >
> > > Sorry, I am not entirely sure I understand your question. It does not
> > > appear that priv needs to be reused.
> >
> > Then why does it exist at all?
> 
> Emm...
> Perhaps I have strayed from the topic of our discussion. Let's try to
> get back on track.
> After replying to your previous email, I thought about whether your
> question was whether we need something similar to
> 
> pci_set_drvdata(dev, priv);
> 
> Looking at the code as a whole, perhaps it is because I am used to
> using `priv` as a function parameter, so in most cases I use `priv`
> directly, except in interrupt routines and INIT_WORK
> I hope we are on the right track with our discussion.

Okay, use priv for now.  But only pass other parameters when they cannot
be derived from priv.  Also ddata tends to be preferred over priv in
this subsystem.

-- 
Lee Jones [李琼斯]

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

[Binbin Zhou]

Hi Lee:

On Wed, Jul 23, 2025 at 4:21 PM Lee Jones <lee@kernel.org> wrote:
>
> On Wed, 23 Jul 2025, Binbin Zhou wrote:
>
> > Hi Lee:
> >
> > Thanks for your reply again.
> >
> > On Wed, Jul 23, 2025 at 3:31 PM Lee Jones <lee@kernel.org> wrote:
> > >
> > > On Mon, 21 Jul 2025, Binbin Zhou wrote:
> > >
> > > > Hi Lee:
> > > >
> > > > Thanks for your reply.
> > > >
> > > >
> > > > On Fri, Jul 18, 2025 at 9:52 PM Lee Jones <lee@kernel.org> wrote:
> > > > >
> > > > > On Fri, 11 Jul 2025, Binbin Zhou wrote:
> > > > >
> > > > > > Hi Lee:
> > > > > >
> > > > > > Thanks for your review.
> > > > > >
> > > > > > On Thu, Jul 10, 2025 at 6:03 PM Lee Jones <lee@kernel.org> wrote:
> > > > > > >
> > > > > > > On Fri, 04 Jul 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.
> > > > > > > >
> > > > > > > > Quick overview of the entire LS2K BMC reset process:
> > > > > > > >
> > > > > > > > There are two types of reset methods: soft reset (BMC-initiated reboot
> > > > > > > > of IPMI reset command) and BMC watchdog reset (watchdog timeout).
> > > > > > > >
> > > > > > > > First, regardless of the method, an interrupt is generated (PCIe interrupt
> > > > > > > > for soft reset/GPIO interrupt for watchdog reset);
> > > > > > > >
> > > > > > > > Second, during the interrupt process, the system enters bmc_reset_work,
> > > > > > > > clears the bus/IO/mem resources of the LS7A PCI-E bridge, waits for the BMC
> > > > > > > > reset to begin, then restores the parent device's PCI configuration space,
> > > > > > > > waits for the BMC reset to complete, and finally restores the BMC PCI
> > > > > > > > configuration space.
> > > > > > > >
> > > > > > > > Display restoration occurs last.
> > > > > > > >
> > > > > > > > Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> > > > > > > > Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> > > > > > > > Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> > > > > > > > Acked-by: Corey Minyard <corey@minyard.net>
> > > > > > > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > > > > > > ---
> > > > > > > >  drivers/mfd/ls2k-bmc-core.c | 328 ++++++++++++++++++++++++++++++++++++
> > > > > > > >  1 file changed, 328 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> > > > > > > > index 50d560a4611c..1ae673f6a196 100644
> > > > > > > > --- a/drivers/mfd/ls2k-bmc-core.c
> > > > > > > > +++ b/drivers/mfd/ls2k-bmc-core.c
> > > > > > > > @@ -10,8 +10,12 @@
> > > > > > > >   */
> > > > > > > >
> > > > > > > >  #include <linux/aperture.h>
> > > > > > > > +#include <linux/bitfield.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>
> > > > > > > > @@ -19,6 +23,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>
> > > > > > > >
> > > > > > > >  /* LS2K BMC resources */
> > > > > > > >  #define LS2K_DISPLAY_RES_START               (SZ_16M + SZ_2M)
> > > > > > > > @@ -29,6 +35,48 @@
> > > > > > > >  #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 LS7A_PCI_CFG_SIZE            0x100
> > > > > > > > +
> > > > > > > > +/* LS7A bridge registers */
> > > > > > > > +#define LS7A_PCIE_PORT_CTL0          0x0
> > > > > > > > +#define LS7A_PCIE_PORT_STS1          0xC
> > > > > > > > +#define LS7A_GEN2_CTL                        0x80C
> > > > > > > > +#define LS7A_SYMBOL_TIMER            0x71C
> > > > > > > > +
> > > > > > > > +/* Bits of LS7A_PCIE_PORT_CTL0 */
> > > > > > > > +#define LS2K_BMC_PCIE_LTSSM_ENABLE   BIT(3)
> > > > > > > > +
> > > > > > > > +/* Bits of LS7A_PCIE_PORT_STS1 */
> > > > > > > > +#define LS2K_BMC_PCIE_LTSSM_STS              GENMASK(5, 0)
> > > > > > > > +#define LS2K_BMC_PCIE_CONNECTED              0x11
> > > > > > > > +
> > > > > > > > +#define LS2K_BMC_PCIE_DELAY_US               1000
> > > > > > > > +#define LS2K_BMC_PCIE_TIMEOUT_US     1000000
> > > > > > > > +
> > > > > > > > +/* Bits of LS7A_GEN2_CTL */
> > > > > > > > +#define LS7A_GEN2_SPEED_CHANG                BIT(17)
> > > > > > > > +#define LS7A_CONF_PHY_TX             BIT(18)
> > > > > > > > +
> > > > > > > > +/* Bits of LS7A_SYMBOL_TIMER */
> > > > > > > > +#define LS7A_MASK_LEN_MATCH          BIT(26)
> > > > > > > > +
> > > > > > > > +/* Interval between interruptions */
> > > > > > > > +#define LS2K_BMC_INT_INTERVAL                (60 * HZ)
> > > > > > > > +
> > > > > > > > +/* Maximum time to wait for U-Boot and DDR to be ready with ms. */
> > > > > > > > +#define LS2K_BMC_RESET_WAIT_TIME     10000
> > > > > > > > +
> > > > > > > > +/* It's an experience value */
> > > > > > > > +#define LS7A_BAR0_CHECK_MAX_TIMES    2000
> > > > > > > > +
> > > > > > > > +#define LS2K_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
> > > > > > > > +
> > > > > > > >  static struct resource ls2k_display_resources[] = {
> > > > > > > >       DEFINE_RES_MEM_NAMED(LS2K_DISPLAY_RES_START, SZ_4M, "simpledrm-res"),
> > > > > > > >  };
> > > > > > > > @@ -62,6 +110,273 @@ static struct mfd_cell ls2k_bmc_cells[] = {
> > > > > > > >       MFD_CELL_RES("ls2k-ipmi-si", ls2k_ipmi4_resources),
> > > > > > > >  };
> > > > > > > >
> > > > > > > > +/* Index of the BMC PCI configuration space to be restored at BMC reset. */
> > > > > > > > +struct ls2k_bmc_pci_data {
> > > > > > > > +     u32 pci_command;
> > > > > > > > +     u32 base_address0;
> > > > > > > > +     u32 interrupt_line;
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +/* Index of the parent PCI configuration space to be restored at BMC reset. */
> > > > > > > > +struct ls2k_bmc_bridge_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;
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +struct ls2k_bmc_pdata {
> > > > > > > > +     struct device *dev;
> > > > > > > > +     struct work_struct bmc_reset_work;
> > > > > > > > +     struct ls2k_bmc_pci_data bmc_pci_data;
> > > > > > > > +     struct ls2k_bmc_bridge_pci_data bridge_pci_data;
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +static bool ls2k_bmc_bar0_addr_is_set(struct pci_dev *ppdev)
> > > > > > >
> > > > > > > Nit: This is usually called pdev.
> > > > > >
> > > > > > OK.
> > > > >
> > > > > Snip things you agree with please.
> > > > >
> > > > > [...]
> > > > >
> > > > > > > > +static void ls2k_bmc_events_fn(struct work_struct *work)
> > > > > > > > +{
> > > > > > > > +     struct ls2k_bmc_pdata *priv = container_of(work, struct ls2k_bmc_pdata, bmc_reset_work);
> > > > > > > > +
> > > > > > > > +     /*
> > > > > > > > +      * The PCI-E is lost when the BMC resets, at which point access to the PCI-E
> > > > > > > > +      * from other CPUs is suspended to prevent a crash.
> > > > > > > > +      */
> > > > > > > > +     stop_machine(ls2k_bmc_recover_pci_data, priv, NULL);
> > > > > > > > +
> > > > > > > > +#ifdef CONFIG_VT
> > > > > > >
> > > > > > > #ifery in C-files is generally frowned upon.
> > > > > > >
> > > > > > > Is the any pieces of run-time data you can use instead?
> > > > > > >
> > > > > > > Or a stub which culminated in a no-op if !CONFIG_VT?
> > > > > >
> > > > > > Emm, I'm not sure if I understand correctly, but is the following way suitable?
> > > > > >
> > > > > >         if (IS_ENABLED(CONFIG_VT))
> > > > >
> > > > > It's better than #if, but even better would be a stub in a header file.
> > > >
> > > > Hmm... The declarations for vt_move_to_console()/set_console() are in
> > > > two VT-related header files [1]. Adding the relevant stub functions
> > > > involves the VT subsystem, which does not seem to be relevant to the
> > > > subject of our patch set.
> > > > Therefore, I think the above modification is more suitable for us.
> > >
> > > All of the subsystems in the kernel are open source, last time I checked. :)
> > >
> > > But okay, I won't insist on it.
> > >
> > > > [1]:
> > > > vt_move_to_console:
> > > > https://elixir.bootlin.com/linux/v6.15/source/include/linux/vt_kern.h#L141
> > > > set_console: https://elixir.bootlin.com/linux/v6.15/source/include/linux/kbd_kern.h#L69
> > > >
> > > > >
> > > > > >                 /* Re-push the display due to previous PCI-E loss. */
> > > > > >                 set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> > > > > >
> > > > > > >
> > > > > > > > +     /* Re-push the display due to previous PCI-E loss. */
> > > > > > > > +     set_console(vt_move_to_console(MAX_NR_CONSOLES - 1, 1));
> > > > > > > > +#endif
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static irqreturn_t ls2k_bmc_interrupt(int irq, void *arg)
> > > > > > > > +{
> > > > > > > > +     struct ls2k_bmc_pdata *priv = arg;
> > > > > > > > +     static unsigned long last_jiffies;
> > > > > > > > +
> > > > > > > > +     if (system_state != SYSTEM_RUNNING)
> > > > > > > > +             return IRQ_HANDLED;
> > > > > > > > +
> > > > > > > > +     /* Skip interrupt in LS2K_BMC_INT_INTERVAL */
> > > > > > > > +     if (time_after(jiffies, last_jiffies + LS2K_BMC_INT_INTERVAL)) {
> > > > > > > > +             schedule_work(&priv->bmc_reset_work);
> > > > > > > > +             last_jiffies = jiffies;
> > > > > > > > +     }
> > > > > > > > +
> > > > > > > > +     return IRQ_HANDLED;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +/*
> > > > > > > > + * Saves the BMC parent device (LS7A) and its own PCI configuration space registers
> > > > > > > > + * that need to be restored after BMC reset.
> > > > > > > > + */
> > > > > > > > +static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > > > > > > > +{
> > > > > > > > +     struct pci_dev *parent = pdev->bus->self;
> > > > > > > > +     int base, i = 0;
> > > > > > > > +
> > > > > > > > +     pci_read_config_dword(parent, PCI_COMMAND, &priv->bridge_pci_data.pci_command);
> > > > > > > > +
> > > > > > > > +     for (base = PCI_BASE_ADDRESS_0; base <= PCI_BASE_ADDRESS_5; base += 4, i++)
> > > > > > > > +             pci_read_config_dword(parent, base, &priv->bridge_pci_data.base_address[i]);
> > > > > > > > +
> > > > > > > > +     pci_read_config_dword(parent, PCI_ROM_ADDRESS, &priv->bridge_pci_data.rom_addreess);
> > > > > > > > +     pci_read_config_dword(parent, PCI_INTERRUPT_LINE, &priv->bridge_pci_data.interrupt_line);
> > > > > > > > +
> > > > > > > > +     pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_LO,
> > > > > > > > +                           &priv->bridge_pci_data.msi_lo);
> > > > > > > > +     pci_read_config_dword(parent, parent->msi_cap + PCI_MSI_ADDRESS_HI,
> > > > > > > > +                           &priv->bridge_pci_data.msi_hi);
> > > > > > > > +
> > > > > > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_DEVCTL,
> > > > > > > > +                           &priv->bridge_pci_data.devctl);
> > > > > > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCAP,
> > > > > > > > +                           &priv->bridge_pci_data.linkcap);
> > > > > > > > +     pci_read_config_dword(parent, parent->pcie_cap + PCI_EXP_LNKCTL,
> > > > > > > > +                           &priv->bridge_pci_data.linkctl_sts);
> > > > > > > > +
> > > > > > > > +     pci_read_config_dword(parent, LS7A_GEN2_CTL, &priv->bridge_pci_data.gen2_ctrl);
> > > > > > > > +     priv->bridge_pci_data.gen2_ctrl |= FIELD_PREP(LS7A_GEN2_SPEED_CHANG, 0x1)
> > > > > > > > +                                     | FIELD_PREP(LS7A_CONF_PHY_TX, 0x0);
> > > > > > > > +
> > > > > > > > +     pci_read_config_dword(parent, LS7A_SYMBOL_TIMER, &priv->bridge_pci_data.symbol_timer);
> > > > > > > > +     priv->bridge_pci_data.symbol_timer |= LS7A_MASK_LEN_MATCH;
> > > > > > > > +
> > > > > > > > +     pci_read_config_dword(pdev, PCI_COMMAND, &priv->bmc_pci_data.pci_command);
> > > > > > > > +     pci_read_config_dword(pdev, PCI_BASE_ADDRESS_0, &priv->bmc_pci_data.base_address0);
> > > > > > > > +     pci_read_config_dword(pdev, PCI_INTERRUPT_LINE, &priv->bmc_pci_data.interrupt_line);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int ls2k_bmc_pdata_initial(struct pci_dev *pdev, struct ls2k_bmc_pdata *priv)
> > > > > > > > +{
> > > > > > > > +     int gsi = 16 + (LS2K_BMC_RESET_GPIO & 7);
> > > > > > > > +     void __iomem *gpio_base;
> > > > > > > > +     int irq, ret;
> > > > > > > > +
> > > > > > > > +     ls2k_bmc_save_pci_data(pdev, priv);
> > > > > > > > +
> > > > > > > > +     INIT_WORK(&priv->bmc_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 PCI-E request_irq(%d) failed\n", pdev->irq);
> > > > > > >
> > > > > > > Please don't use function names in error messages.
> > > > > > >
> > > > > > > Make them human readable inclusive of non-kernel engineers.
> > > > > >
> > > > > > How about:
> > > > > >
> > > > > > dev_err(ddata->dev, "Failed to request LS2KBMC PCI-E IRQ %d.\n", pdev->irq);
> > > > > >
> > > > > > also, the error message regarding GPIO IRQ is as follows:
> > > > > >
> > > > > > dev_err(ddata->dev, "Failed to request LS2KBMC GPIO IRQ %d.\n", irq);
> > > > >
> > > > > Yes, much better.
> > > > >
> > > > > [...]
> > > > >
> > > > > > > > +     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);
> > > > > > >
> > > > > > > priv (ddata) already contains dev - you don't need both.
> > > > > >
> > > > > > Yes, we just pass priv(ddata), and
> > > > > >
> > > > > > struct pci_dev *pdev = to_pci_dev(ddata->dev);
> > > > >
> > > > > I would pass dev ... hold on, where do you store priv for reuse?
> > > >
> > > > Sorry, I am not entirely sure I understand your question. It does not
> > > > appear that priv needs to be reused.
> > >
> > > Then why does it exist at all?
> >
> > Emm...
> > Perhaps I have strayed from the topic of our discussion. Let's try to
> > get back on track.
> > After replying to your previous email, I thought about whether your
> > question was whether we need something similar to
> >
> > pci_set_drvdata(dev, priv);
> >
> > Looking at the code as a whole, perhaps it is because I am used to
> > using `priv` as a function parameter, so in most cases I use `priv`
> > directly, except in interrupt routines and INIT_WORK
> > I hope we are on the right track with our discussion.
>
> Okay, use priv for now.  But only pass other parameters when they cannot
> be derived from priv.  Also ddata tends to be preferred over priv in
> this subsystem.

Okay, if you don't mind me using `priv(ddata)` as the parameter for
ls2k_bmc_pdata_initial(), then my V8 patchset[1] has addressed all of
your comments in V7.

[1]: https://lore.kernel.org/all/cover.1752548073.git.zhoubinbin@loongson.cn/

>
> --
> Lee Jones [李琼斯]

-- 
Thanks.
Binbin