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

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

[Miao Wang via B4 Relay]

Previously, the driver has been introduced to support the Loongson 2K
BMC running on the Loongson Servers, which is essential to prevent
the system from hanging when the BMC is being reset and the default
efi-framebuffer is being used. However, there are some drawbacks in the
driver.

Firstly, the driver tries to read and write to the connected PCI-E host
controller registers, assuming that the BMC is connected to LS7A PCI-E
host controller. This assumption should be true for real products, but
to prevent from accidentally reading and writing to the wrong PCI-E host
controller, this driver should be modified to check this before
accessing the registers.

Secondly, the driver uses non-exported functions to tell the vt
subsystem to redraw the screen, preventing the driver from being
compiling as a module. This can be fixed by using the exported
functions instead.

Thirdly, the driver directly accesses the GPIO controller registers
using hard-coded addresses, which might conflict with the loaded GPIO
controller driver for the same GPIO controller. This is fixed in this
series by using the GPIO subsystem APIs instead. However, legacy GPIO
APIs have to be used in this fixed to correctly request a GPIO
descriptor from the GPIO subsystem, which might be further discussed
to find a better solution.

Finally, there is a minor issue in the driver where it changes the
mode string describing the screen resolution during probing, which
prevents the device from being probed again if -EPROBE_DEFER is
returned by the probe function.

I have tested the changes in this series on a single-socket Loongson
3C6000 server with a Loongson 2K BMC, and the driver works as expected
when the corresponding GPIO driver is additionally loaded.

Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
Changes in v3:
- Check the return value of devm_add_action_or_reset when registering
  the cleanup hook of the work queue
- Use swnode to create the link between the device to the GPIO chip,
  and prevent borrowing the legacy GPIO APIs
- Link to v2: https://lore.kernel.org/r/20260708-ls2kbmc-mod-v2-0-2afdd1741766@gmail.com

Changes in v2:
- Several fixes suggested by the Sashiko AI review bot
- Add a cleanup function for the wq on removal of the device
- Relax the reverse dependency from CONFIG_IPMI_LS2K to
  CONFIG_MFD_LS2K_BMC_CORE to allow the driver to be built as a module
- Link to v1: https://lore.kernel.org/r/20260708-ls2kbmc-mod-v1-0-c344bf5defa3@gmail.com

---
Miao Wang (7):
      mfd: ls2kbmc: Make a copy when parsing mode string
      mfd: ls2kbmc: Sanity check for the connected pci port
      mfd: ls2kbmc: Redraw using exported functions
      mfd: ls2kbmc: Cancel the work queue on removal
      ipmi: ls2k: Relax the dependency to its mfd driver
      mfd: ls2kbmc: Able to be compiled as a module
      mfd: ls2kbmc: Capture the reset event of BMC through GPIO

 drivers/char/ipmi/Kconfig   |   2 +-
 drivers/mfd/Kconfig         |   2 +-
 drivers/mfd/ls2k-bmc-core.c | 235 ++++++++++++++++++++++++++++++++++----------
 3 files changed, 184 insertions(+), 55 deletions(-)
---
base-commit: 0e35b9b6ec0ffcc5e23cbdec09f5c622ad532b53
change-id: 20260626-ls2kbmc-mod-5209193009b2

Best regards,
-- 
Miao Wang <shankerwangmiao@gmail.com>

[PATCH RFC v3 1/7] mfd: ls2kbmc: Make a copy when parsing mode string

[Miao Wang via B4 Relay]

From: Miao Wang <shankerwangmiao@gmail.com>

When parsing the mode string from BMC, the string is manipulated
in-place with strsep(), preventing from parsing it again. Make a copy of
the original string and manipulate the copy instead to fix this.

Fixes: 0d64f6d1ffe9 ("mfd: ls2kbmc: Introduce Loongson-2K BMC core driver")
Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
 drivers/mfd/ls2k-bmc-core.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
index 408056bfb2fe757a5bde43775a483a48352e706d..27f6e096404d67459038a0607378057ec7ef69ab 100644
--- a/drivers/mfd/ls2k-bmc-core.c
+++ b/drivers/mfd/ls2k-bmc-core.c
@@ -427,34 +427,54 @@ static int ls2k_bmc_init(struct ls2k_bmc_ddata *ddata)
  */
 static int ls2k_bmc_parse_mode(struct pci_dev *pdev, struct simplefb_platform_data *pd)
 {
-	char *mode;
+	/* Assume 64 bytes is enough for the resolution string */
+	char mode_buf[64], mode_buf_orig[64];
+	char *mode = mode_buf;
+	const void __iomem *mode_base;
 	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)
+	mode_base = ioremap(pci_resource_start(pdev, 0) + SZ_16M,
+			    sizeof(mode_buf));
+	if (!mode_base)
 		return -ENOMEM;
+	memcpy_fromio(mode_buf, mode_base, sizeof(mode_buf) - 1);
+	mode_buf[sizeof(mode_buf) - 1] = '\0';
+	iounmap((void __iomem *)mode_base);
+	memcpy(mode_buf_orig, mode_buf, sizeof(mode_buf_orig));
 
 	/* The resolution field starts with the flag "video=". */
 	if (!strncmp(mode, "video=", 6))
 		mode = mode + 6;
+	else
+		goto invalid_mode;
 
-	ret = kstrtoint(strsep(&mode, "x"), 10, &pd->width);
+	ret = kstrtouint(strsep(&mode, "x"), 10, &pd->width);
 	if (ret)
 		return ret;
+	if (mode == NULL)
+		goto invalid_mode;
 
-	ret = kstrtoint(strsep(&mode, "-"), 10, &pd->height);
+	ret = kstrtouint(strsep(&mode, "-"), 10, &pd->height);
 	if (ret)
 		return ret;
+	if (mode == NULL)
+		goto invalid_mode;
 
 	ret = kstrtoint(strsep(&mode, "@"), 10, &depth);
 	if (ret)
 		return ret;
+	if (mode == NULL)
+		goto invalid_mode;
 
 	pd->stride = pd->width * depth / 8;
 	pd->format = depth == 32 ? "a8r8g8b8" : "r5g6b5";
 
 	return 0;
+
+invalid_mode:
+	dev_err(&pdev->dev, "Invalid resolution string: %s\n", mode_buf_orig);
+	return -EINVAL;
 }
 
 static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)

-- 
2.49.0

[PATCH RFC v3 2/7] mfd: ls2kbmc: Sanity check for the connected pci port

[Miao Wang via B4 Relay]

From: Miao Wang <shankerwangmiao@gmail.com>

When the bmc resets, the recovery procedure require to reconfigure the
parent device. The driver assumes that the parent device should be LS7A.
Add a sanity check on initialization to ensure this and prevent from
accidentally operating on non-LS7A ports.

Fixes: d952bba3fbb5 ("mfd: ls2kbmc: Add Loongson-2K BMC reset function support")
Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
 drivers/mfd/ls2k-bmc-core.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
index 27f6e096404d67459038a0607378057ec7ef69ab..b02e4955e9b04f517892a18b2ef103b5e481a238 100644
--- a/drivers/mfd/ls2k-bmc-core.c
+++ b/drivers/mfd/ls2k-bmc-core.c
@@ -35,6 +35,15 @@
 #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)
 
+/* LS7A port Device IDs */
+#define DEV_LS7A1K_PCIE_PORT0	0x7a09
+#define DEV_LS7A1K_PCIE_PORT1	0x7a19
+#define DEV_LS7A1K_PCIE_PORT2	0x7a29
+#define DEV_LS7A2K_PCIE_PORT0	0x7a39
+#define DEV_LS7A2K_PCIE_PORT1	0x7a49
+#define DEV_LS7A2K_PCIE_PORT2	0x7a59
+#define DEV_LS7A2K_PCIE_PORT3	0x7a69
+
 #define LS7A_PCI_CFG_SIZE		0x100
 
 /* LS7A bridge registers */
@@ -477,6 +486,24 @@ static int ls2k_bmc_parse_mode(struct pci_dev *pdev, struct simplefb_platform_da
 	return -EINVAL;
 }
 
+static const struct pci_device_id ls7a_ports[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, DEV_LS7A1K_PCIE_PORT0) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, DEV_LS7A1K_PCIE_PORT1) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, DEV_LS7A1K_PCIE_PORT2) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, DEV_LS7A2K_PCIE_PORT0) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, DEV_LS7A2K_PCIE_PORT1) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, DEV_LS7A2K_PCIE_PORT2) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, DEV_LS7A2K_PCIE_PORT3) },
+	{ }
+};
+
+static bool ls2k_check_parent(struct pci_dev *dev)
+{
+	struct pci_dev *parent = dev->bus->self;
+
+	return parent && pci_match_id(ls7a_ports, parent) != NULL;
+}
+
 static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	struct simplefb_platform_data pd;
@@ -488,6 +515,11 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (ret)
 		return ret;
 
+	if (!ls2k_check_parent(dev)) {
+		dev_err(&dev->dev, "Expected to be connected to LS7A PCI-E port\n");
+		return -ENODEV;
+	}
+
 	ddata = devm_kzalloc(&dev->dev, sizeof(*ddata), GFP_KERNEL);
 	if (!ddata)
 		return -ENOMEM;

-- 
2.49.0

[PATCH RFC v3 3/7] mfd: ls2kbmc: Redraw using exported functions

[Miao Wang via B4 Relay]

From: Miao Wang <shankerwangmiao@gmail.com>

Use update_screen, i.e. redraw_screen() to trigger the redraw of the
current vt.

Fixes: d952bba3fbb5 ("mfd: ls2kbmc: Add Loongson-2K BMC reset function support")
Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
 drivers/mfd/ls2k-bmc-core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
index b02e4955e9b04f517892a18b2ef103b5e481a238..a5465c42a77b8b7b81e3ad787d6036679c6ba6df 100644
--- a/drivers/mfd/ls2k-bmc-core.c
+++ b/drivers/mfd/ls2k-bmc-core.c
@@ -25,6 +25,7 @@
 #include <linux/platform_device.h>
 #include <linux/stop_machine.h>
 #include <linux/vt_kern.h>
+#include <linux/console.h>
 
 /* LS2K BMC resources */
 #define LS2K_DISPLAY_RES_START		(SZ_16M + SZ_2M)
@@ -310,7 +311,9 @@ static void ls2k_bmc_events_fn(struct work_struct *work)
 
 	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));
+		console_lock();
+		update_screen(vc_cons[fg_console].d);
+		console_unlock();
 	}
 }
 

-- 
2.49.0

[PATCH RFC v3 4/7] mfd: ls2kbmc: Cancel the work queue on removal

[Miao Wang via B4 Relay]

From: Miao Wang <shankerwangmiao@gmail.com>

When the device is being removeed, the work queue should be canceled to
avoid any pending work to be executed after the device is removed.

Fixes: d952bba3fbb5 ("mfd: ls2kbmc: Add Loongson-2K BMC reset function support")
Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
 drivers/mfd/ls2k-bmc-core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
index a5465c42a77b8b7b81e3ad787d6036679c6ba6df..1466b314fc4e577fe5e31404444648b5b0447ebb 100644
--- a/drivers/mfd/ls2k-bmc-core.c
+++ b/drivers/mfd/ls2k-bmc-core.c
@@ -375,6 +375,12 @@ static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_ddata *
 	pci_read_config_dword(pdev, PCI_INTERRUPT_LINE, &ddata->bmc_pci_data.interrupt_line);
 }
 
+static void ls2k_bmc_cancel_wq(void *data)
+{
+	struct ls2k_bmc_ddata *ddata = data;
+	(void) cancel_work_sync(&ddata->bmc_reset_work);
+}
+
 static int ls2k_bmc_init(struct ls2k_bmc_ddata *ddata)
 {
 	struct pci_dev *pdev = to_pci_dev(ddata->dev);
@@ -385,6 +391,10 @@ static int ls2k_bmc_init(struct ls2k_bmc_ddata *ddata)
 
 	INIT_WORK(&ddata->bmc_reset_work, ls2k_bmc_events_fn);
 
+	ret = devm_add_action_or_reset(ddata->dev, ls2k_bmc_cancel_wq, ddata);
+	if (ret)
+		return ret;
+
 	ret = devm_request_irq(&pdev->dev, pdev->irq, ls2k_bmc_interrupt,
 			       IRQF_SHARED | IRQF_TRIGGER_FALLING, "ls2kbmc pcie", ddata);
 	if (ret) {

-- 
2.49.0

[PATCH RFC v3 5/7] ipmi: ls2k: Relax the dependency to its mfd driver

[Miao Wang via B4 Relay]

From: Miao Wang <shankerwangmiao@gmail.com>

There is no strong dependency between the IPMI driver and its mfd
driver. Although the IPMI driver will not work without the mfd driver,
it is not a hard dependency. The IPMI driver can actually be compiled
without the mfd driver, and it will just fail to probe. When the mfd
driver is loaded, the IPMI driver will probe successfully. Therefore,
the dependency of the IPMI driver on its mfd driver should be relaxed
to "imply" from "select". This will allow the mfd driver to be compiled
as a module and the IPMI driver to be compiled as a part of the ipmi_si
module.

Fixes: d46651d4e3c0 ("ipmi: Add Loongson-2K BMC support")
Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
 drivers/char/ipmi/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
index 669f7600019747bcd2b37563477cf336f19a0407..f456e3e89416932f0d21bb742153a503aeb24267 100644
--- a/drivers/char/ipmi/Kconfig
+++ b/drivers/char/ipmi/Kconfig
@@ -87,7 +87,7 @@ config IPMI_IPMB
 config IPMI_LS2K
 	bool 'Loongson-2K IPMI interface'
 	depends on LOONGARCH
-	select MFD_LS2K_BMC_CORE
+	imply MFD_LS2K_BMC_CORE
 	help
 	  Provides a driver for Loongson-2K IPMI interfaces.
 

-- 
2.49.0

[PATCH RFC v3 6/7] mfd: ls2kbmc: Able to be compiled as a module

[Miao Wang via B4 Relay]

From: Miao Wang <shankerwangmiao@gmail.com>

Make ls2kmc able to be compiled as a module

Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
 drivers/mfd/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 763ce6a34782bdd5d2b1a4d840c75b040092d83e..a7a9f97af248c88489dc1203a1ba05f2ce4827df 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2494,7 +2494,7 @@ config MFD_LOONGSON_SE
 	  which will forward them to the corresponding engine.
 
 config MFD_LS2K_BMC_CORE
-	bool "Loongson-2K Board Management Controller Support"
+	tristate "Loongson-2K Board Management Controller Support"
 	depends on PCI && ACPI_GENERIC_GSI
 	select MFD_CORE
 	help

-- 
2.49.0

[PATCH RFC v3 7/7] mfd: ls2kbmc: Capture the reset event of BMC through GPIO

[Miao Wang via B4 Relay]

From: Miao Wang <shankerwangmiao@gmail.com>

The reset event of BMC is captured through GPIO. However, this driver
bypasses the GPIO framework and directly accesses the GPIO controller
through the fixed address. When the same GPIO controller is also
exposed through ACPI and probed by the corresponding GPIO driver,
there would be a conflict between the two drivers.

This patch will try to find the GPIO through declared GPIO pin in the
_CRS resources of the ACPI node. If no such delaration is found, the
driver will fall back to search for the correct GPIO controller and pin
according to the fixed address and pin number. A possible DSDT
declaration for the GPIO pin might be as follows:

    Device (BMC0) {
        Name (_ADR, ...) // Match the PCI address of the BMC device
        // \_SB.GPO1 is the ACPI path of the GPIO controller
        Name (_CRS, ResourceTemplate () {
            GpioInt (Edge, ActiveLow, Exclusive, PullNone, 0,
                     "\\_SB.GPO1", 0) {
                14 // 14 is the GPIO pin number
            }
    }

Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
 drivers/mfd/ls2k-bmc-core.c | 158 +++++++++++++++++++++++++++++++-------------
 1 file changed, 111 insertions(+), 47 deletions(-)

diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
index 1466b314fc4e577fe5e31404444648b5b0447ebb..5a2644ff07fee05f4c8cbc54cb62abc2358c1820 100644
--- a/drivers/mfd/ls2k-bmc-core.c
+++ b/drivers/mfd/ls2k-bmc-core.c
@@ -26,6 +26,10 @@
 #include <linux/stop_machine.h>
 #include <linux/vt_kern.h>
 #include <linux/console.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/gpio/property.h>
+#include <linux/gpio/machine.h>
 
 /* LS2K BMC resources */
 #define LS2K_DISPLAY_RES_START		(SZ_16M + SZ_2M)
@@ -81,18 +85,6 @@
 
 #define PCI_REG_STRIDE			0x4
 
-#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
-
-#define LOONGSON_IO_INT_BASE		16
-#define LS2K_BMC_RESET_GPIO_INT_VEC	(LS2K_BMC_RESET_GPIO % 8)
-#define LS2K_BMC_RESET_GPIO_GSI		(LOONGSON_IO_INT_BASE + LS2K_BMC_RESET_GPIO_INT_VEC)
-
 enum {
 	LS2K_BMC_DISPLAY,
 	LS2K_BMC_IPMI0,
@@ -186,6 +178,7 @@ struct ls2k_bmc_ddata {
 	struct work_struct bmc_reset_work;
 	struct ls2k_bmc_pci_data bmc_pci_data;
 	struct ls2k_bmc_bridge_pci_data bridge_pci_data;
+	struct gpio_desc *reset_gpio;
 };
 
 static bool ls2k_bmc_bar0_addr_is_set(struct pci_dev *pdev)
@@ -375,6 +368,90 @@ static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_ddata *
 	pci_read_config_dword(pdev, PCI_INTERRUPT_LINE, &ddata->bmc_pci_data.interrupt_line);
 }
 
+static struct fwnode_handle *gpio_chip_get_fwnode(struct gpio_chip *chip)
+{
+	if (chip->fwnode)
+		return chip->fwnode;
+	else if (chip->parent)
+		return chip->parent->fwnode;
+	else
+		return NULL;
+}
+
+static int ls2k_bmc_gpiochip_find(struct gpio_chip *gc, const void *data)
+{
+	struct acpi_device *adev;
+	struct list_head resource_list;
+	struct resource_entry *rentry;
+	struct fwnode_handle *fwnode = gpio_chip_get_fwnode(gc);
+	phys_addr_t start_addr = (phys_addr_t) data;
+	int ret, found = 0;
+
+	if (!is_acpi_node(fwnode))
+		goto out;
+
+	adev = to_acpi_device_node(fwnode);
+	if (!adev)
+		goto out;
+
+	INIT_LIST_HEAD(&resource_list);
+
+	ret = acpi_dev_get_memory_resources(adev, &resource_list);
+	if (ret < 0)
+		goto out;
+	rentry = list_first_entry_or_null(&resource_list, struct resource_entry, node);
+	if (!rentry)
+		goto free_resource_list;
+	if (rentry->res->start == start_addr)
+		found = 1;
+
+free_resource_list:
+	acpi_dev_free_resource_list(&resource_list);
+out:
+	return found;
+}
+
+static struct gpio_desc *ls2k_bmc_find_gpio(struct ls2k_bmc_ddata *ddata)
+{
+	/*
+	 * In conventional way, the GPIO should be obtained through ACPI or
+	 * device tree. However, when the information is not available,
+	 * we should find the GPIO according to the convention of the server
+	 * boards with LS2K BMC, the gpio signal relelecting the reset event
+	 * of the BMC should be connected to pin 14 of the GPIO input of
+	 * the first CPU node. The address of that GPIO controller is fixed.
+	 */
+	static const phys_addr_t LOONGSON_GPIO_REG_BASE = 0x1FE00500;
+	static const unsigned int LS2K_BMC_RESET_GPIO = 14;
+	int ret;
+	struct gpio_device *gdev __free(gpio_device_put) = NULL;
+	struct gpio_chip *gc;
+	struct property_entry ls2k_bmc_swnode_properties[2] = { 0 };
+
+	dev_dbg(ddata->dev, "Searching for GPIO chip at address %pa\n", &LOONGSON_GPIO_REG_BASE);
+
+	gdev = gpio_device_find((void *)LOONGSON_GPIO_REG_BASE, ls2k_bmc_gpiochip_find);
+
+	if (!gdev) {
+		dev_dbg(ddata->dev, "cannot find GPIO chip at address %pa, deferring\n",
+			&LOONGSON_GPIO_REG_BASE);
+		return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	gc = gpio_device_get_chip(gdev);
+
+	ls2k_bmc_swnode_properties[0] = PROPERTY_ENTRY_GPIO("gpio",
+		gpio_chip_get_fwnode(gc), LS2K_BMC_RESET_GPIO, GPIO_ACTIVE_HIGH);
+
+	ret = device_create_managed_software_node(ddata->dev, ls2k_bmc_swnode_properties, NULL);
+	if (ret) {
+		dev_err(ddata->dev, "Failed to create software node for GPIO reset: %d\n", ret);
+		return ERR_PTR(ret);
+	}
+
+	return devm_gpiod_get_index(ddata->dev, NULL, 0, GPIOD_IN);
+}
+
 static void ls2k_bmc_cancel_wq(void *data)
 {
 	struct ls2k_bmc_ddata *ddata = data;
@@ -384,8 +461,7 @@ static void ls2k_bmc_cancel_wq(void *data)
 static int ls2k_bmc_init(struct ls2k_bmc_ddata *ddata)
 {
 	struct pci_dev *pdev = to_pci_dev(ddata->dev);
-	void __iomem *gpio_base;
-	int gpio_irq, ret, val;
+	int gpio_irq, ret;
 
 	ls2k_bmc_save_pci_data(pdev, ddata);
 
@@ -402,44 +478,32 @@ static int ls2k_bmc_init(struct ls2k_bmc_ddata *ddata)
 		return ret;
 	}
 
-	gpio_base = ioremap(LOONGSON_GPIO_REG_BASE, LOONGSON_GPIO_REG_SIZE);
-	if (!gpio_base)
-		return -ENOMEM;
-
-	/* Disable GPIO output */
-	val = readl(gpio_base + LOONGSON_GPIO_OEN);
-	writel(val | BIT(LS2K_BMC_RESET_GPIO), gpio_base + LOONGSON_GPIO_OEN);
-
-	/* Enable GPIO functionality */
-	val = readl(gpio_base + LOONGSON_GPIO_FUNC);
-	writel(val & ~BIT(LS2K_BMC_RESET_GPIO), gpio_base + LOONGSON_GPIO_FUNC);
-
-	/* Set GPIO interrupts to low-level active */
-	val = readl(gpio_base + LOONGSON_GPIO_INTPOL);
-	writel(val & ~BIT(LS2K_BMC_RESET_GPIO), gpio_base + LOONGSON_GPIO_INTPOL);
-
-	/* Enable GPIO interrupts */
-	val = readl(gpio_base + LOONGSON_GPIO_INTEN);
-	writel(val | BIT(LS2K_BMC_RESET_GPIO), gpio_base + LOONGSON_GPIO_INTEN);
+	ddata->reset_gpio = devm_gpiod_get_index_optional(&pdev->dev, NULL, 0, GPIOD_IN);
+	if (IS_ERR(ddata->reset_gpio)) {
+		ret = PTR_ERR(ddata->reset_gpio);
+		ddata->reset_gpio = NULL;
+		return dev_err_probe(ddata->dev, ret, "Failed to get GPIO pin for reset signal\n");
+	}
+	if (ddata->reset_gpio == NULL) {
+		ddata->reset_gpio = ls2k_bmc_find_gpio(ddata);
+		if (IS_ERR(ddata->reset_gpio)) {
+			ret = PTR_ERR(ddata->reset_gpio);
+			ddata->reset_gpio = NULL;
+			return dev_err_probe(ddata->dev, ret, "Failed to find GPIO pin for reset signal\n");
+		}
+	}
 
-	iounmap(gpio_base);
+	gpio_irq = gpiod_to_irq(ddata->reset_gpio);
 
-	/*
-	 * 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.
-	 */
-	gpio_irq = acpi_register_gsi(NULL, LS2K_BMC_RESET_GPIO_GSI, ACPI_EDGE_SENSITIVE,
-				     ACPI_ACTIVE_LOW);
 	if (gpio_irq < 0)
-		return gpio_irq;
+		return dev_err_probe(ddata->dev, gpio_irq, "Failed to get IRQ for GPIO reset signal input\n");
 
-	ret = devm_request_irq(ddata->dev, gpio_irq, ls2k_bmc_interrupt,
-			       IRQF_SHARED | IRQF_TRIGGER_FALLING, "ls2kbmc gpio", ddata);
-	if (ret)
-		dev_err(ddata->dev, "Failed to request LS2KBMC GPIO IRQ %d.\n", gpio_irq);
+	ret = devm_request_irq(&pdev->dev, gpio_irq, ls2k_bmc_interrupt,
+			       IRQF_SHARED | IRQF_TRIGGER_FALLING, "ls2kbmc reset", ddata);
+
+	if (ret != 0)
+		return dev_err_probe(ddata->dev, ret, "Failed to request IRQ %d for GPIO reset signal input.\n", gpio_irq);
 
-	acpi_unregister_gsi(LS2K_BMC_RESET_GPIO_GSI);
 	return ret;
 }
 

-- 
2.49.0

Re: [PATCH RFC v3 1/7] mfd: ls2kbmc: Make a copy when parsing mode string

[Lee Jones]

Why is this v3 set still RFC?  What comment are you requesting?

On Fri, 10 Jul 2026, Miao Wang via B4 Relay wrote:

> From: Miao Wang <shankerwangmiao@gmail.com>
> 
> When parsing the mode string from BMC, the string is manipulated
> in-place with strsep(), preventing from parsing it again. Make a copy of
> the original string and manipulate the copy instead to fix this.
> 
> Fixes: 0d64f6d1ffe9 ("mfd: ls2kbmc: Introduce Loongson-2K BMC core driver")
> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
> ---
>  drivers/mfd/ls2k-bmc-core.c | 30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> index 408056bfb2fe757a5bde43775a483a48352e706d..27f6e096404d67459038a0607378057ec7ef69ab 100644
> --- a/drivers/mfd/ls2k-bmc-core.c
> +++ b/drivers/mfd/ls2k-bmc-core.c
> @@ -427,34 +427,54 @@ static int ls2k_bmc_init(struct ls2k_bmc_ddata *ddata)
>   */
>  static int ls2k_bmc_parse_mode(struct pci_dev *pdev, struct simplefb_platform_data *pd)
>  {
> -	char *mode;
> +	/* Assume 64 bytes is enough for the resolution string */
> +	char mode_buf[64], mode_buf_orig[64];
> +	char *mode = mode_buf;
> +	const void __iomem *mode_base;
>  	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)
> +	mode_base = ioremap(pci_resource_start(pdev, 0) + SZ_16M,
> +			    sizeof(mode_buf));
> +	if (!mode_base)
>  		return -ENOMEM;
> +	memcpy_fromio(mode_buf, mode_base, sizeof(mode_buf) - 1);
> +	mode_buf[sizeof(mode_buf) - 1] = '\0';
> +	iounmap((void __iomem *)mode_base);
> +	memcpy(mode_buf_orig, mode_buf, sizeof(mode_buf_orig));
>  
>  	/* The resolution field starts with the flag "video=". */
>  	if (!strncmp(mode, "video=", 6))
>  		mode = mode + 6;
> +	else
> +		goto invalid_mode;
>  
> -	ret = kstrtoint(strsep(&mode, "x"), 10, &pd->width);
> +	ret = kstrtouint(strsep(&mode, "x"), 10, &pd->width);
>  	if (ret)
>  		return ret;
> +	if (mode == NULL)
> +		goto invalid_mode;
>  
> -	ret = kstrtoint(strsep(&mode, "-"), 10, &pd->height);
> +	ret = kstrtouint(strsep(&mode, "-"), 10, &pd->height);
>  	if (ret)
>  		return ret;
> +	if (mode == NULL)
> +		goto invalid_mode;
>  
>  	ret = kstrtoint(strsep(&mode, "@"), 10, &depth);
>  	if (ret)
>  		return ret;
> +	if (mode == NULL)
> +		goto invalid_mode;
>  
>  	pd->stride = pd->width * depth / 8;
>  	pd->format = depth == 32 ? "a8r8g8b8" : "r5g6b5";
>  
>  	return 0;
> +
> +invalid_mode:
> +	dev_err(&pdev->dev, "Invalid resolution string: %s\n", mode_buf_orig);
> +	return -EINVAL;
>  }
>  
>  static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
> 
> -- 
> 2.49.0
> 
> 

-- 
Lee Jones

Re: [PATCH RFC v3 1/7] mfd: ls2kbmc: Make a copy when parsing mode string

[Miao Wang]

Hi,

> 2026年7月23日 19:29，Lee Jones <lee@kernel.org> 写道：
> 
> Why is this v3 set still RFC?  What comment are you requesting?

The reason why this is still in RFC is that I am still waiting
for response and discussion from Loongson personals on the
proposal that adding a DSDT entry for the ls2kbmc device that
describes the GPIO pin associated with the device to signal
the reset event of that BMC device. As pointed by Ruoyao,
such a proposal will need to be discussed with Loongson. I have
CC-ed the patch to the relevant maintainers from Loongson but
failed to receive any response.

Moreover, I am also in need of comments on the backup method
of searching the GPIO device and pin when such a DSDT entry
is not available.

Cheers,

Miao Wang

Re: [PATCH RFC v3 1/7] mfd: ls2kbmc: Make a copy when parsing mode string

[Miao Wang]

> 2026年7月23日 20:02，Miao Wang <shankerwangmiao@gmail.com> 写道：
> 
> Hi,
> 
>> 2026年7月23日 19:29，Lee Jones <lee@kernel.org> 写道：
>> 
>> Why is this v3 set still RFC?  What comment are you requesting?
> 
> The reason why this is still in RFC is that I am still waiting
> for response and discussion from Loongson personals on the
> proposal that adding a DSDT entry for the ls2kbmc device that
> describes the GPIO pin associated with the device to signal
> the reset event of that BMC device. As pointed by Ruoyao,
> such a proposal will need to be discussed with Loongson. I have
> CC-ed the patch to the relevant maintainers from Loongson but
> failed to receive any response.
> 
> Moreover, I am also in need of comments on the backup method
> of searching the GPIO device and pin when such a DSDT entry
> is not available.
> 

To complement, the part that I'm in need of comments is the
7-th patch of this series. The first 6 patches are focusing on
fixing the existing issues in this driver. I thought that they
could be managed to be applied altogether so that the driver
would become a good, ready-to-use state. If the first 6 patches
are good to be applied and we still receive no response from
Loongson personnels, can I split the series into 2 parts?

Cheers,

Miao Wang

Re: [PATCH RFC v3 1/7] mfd: ls2kbmc: Make a copy when parsing mode string

[Lee Jones]

On Thu, 23 Jul 2026, Miao Wang wrote:

> 
> 
> > 2026年7月23日 20:02，Miao Wang <shankerwangmiao@gmail.com> 写道：
> > 
> > Hi,
> > 
> >> 2026年7月23日 19:29，Lee Jones <lee@kernel.org> 写道：
> >> 
> >> Why is this v3 set still RFC?  What comment are you requesting?
> > 
> > The reason why this is still in RFC is that I am still waiting
> > for response and discussion from Loongson personals on the
> > proposal that adding a DSDT entry for the ls2kbmc device that
> > describes the GPIO pin associated with the device to signal
> > the reset event of that BMC device. As pointed by Ruoyao,
> > such a proposal will need to be discussed with Loongson. I have
> > CC-ed the patch to the relevant maintainers from Loongson but
> > failed to receive any response.
> > 
> > Moreover, I am also in need of comments on the backup method
> > of searching the GPIO device and pin when such a DSDT entry
> > is not available.
> > 
> 
> To complement, the part that I'm in need of comments is the
> 7-th patch of this series. The first 6 patches are focusing on
> fixing the existing issues in this driver. I thought that they
> could be managed to be applied altogether so that the driver
> would become a good, ready-to-use state. If the first 6 patches
> are good to be applied and we still receive no response from
> Loongson personnels, can I split the series into 2 parts?

I don't see any reason why not.

-- 
Lee Jones

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

[Huacai Chen]

Hi, Miao,

On Fri, Jul 10, 2026 at 1:24 AM Miao Wang via B4 Relay
<devnull+shankerwangmiao.gmail.com@kernel.org> wrote:
>
> Previously, the driver has been introduced to support the Loongson 2K
> BMC running on the Loongson Servers, which is essential to prevent
> the system from hanging when the BMC is being reset and the default
> efi-framebuffer is being used. However, there are some drawbacks in the
> driver.
>
> Firstly, the driver tries to read and write to the connected PCI-E host
> controller registers, assuming that the BMC is connected to LS7A PCI-E
> host controller. This assumption should be true for real products, but
> to prevent from accidentally reading and writing to the wrong PCI-E host
> controller, this driver should be modified to check this before
> accessing the registers.
>
> Secondly, the driver uses non-exported functions to tell the vt
> subsystem to redraw the screen, preventing the driver from being
> compiling as a module. This can be fixed by using the exported
> functions instead.
You can replace the redraw function, but I don't think it is necessary
to make the bmc driver modular.

BMC core, IPMI and simpledrm display are usually (if not always)
supposed to work as early as possible.


Huacai

>
> Thirdly, the driver directly accesses the GPIO controller registers
> using hard-coded addresses, which might conflict with the loaded GPIO
> controller driver for the same GPIO controller. This is fixed in this
> series by using the GPIO subsystem APIs instead. However, legacy GPIO
> APIs have to be used in this fixed to correctly request a GPIO
> descriptor from the GPIO subsystem, which might be further discussed
> to find a better solution.
>
> Finally, there is a minor issue in the driver where it changes the
> mode string describing the screen resolution during probing, which
> prevents the device from being probed again if -EPROBE_DEFER is
> returned by the probe function.
>
> I have tested the changes in this series on a single-socket Loongson
> 3C6000 server with a Loongson 2K BMC, and the driver works as expected
> when the corresponding GPIO driver is additionally loaded.
>
> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
> ---
> Changes in v3:
> - Check the return value of devm_add_action_or_reset when registering
>   the cleanup hook of the work queue
> - Use swnode to create the link between the device to the GPIO chip,
>   and prevent borrowing the legacy GPIO APIs
> - Link to v2: https://lore.kernel.org/r/20260708-ls2kbmc-mod-v2-0-2afdd1741766@gmail.com
>
> Changes in v2:
> - Several fixes suggested by the Sashiko AI review bot
> - Add a cleanup function for the wq on removal of the device
> - Relax the reverse dependency from CONFIG_IPMI_LS2K to
>   CONFIG_MFD_LS2K_BMC_CORE to allow the driver to be built as a module
> - Link to v1: https://lore.kernel.org/r/20260708-ls2kbmc-mod-v1-0-c344bf5defa3@gmail.com
>
> ---
> Miao Wang (7):
>       mfd: ls2kbmc: Make a copy when parsing mode string
>       mfd: ls2kbmc: Sanity check for the connected pci port
>       mfd: ls2kbmc: Redraw using exported functions
>       mfd: ls2kbmc: Cancel the work queue on removal
>       ipmi: ls2k: Relax the dependency to its mfd driver
>       mfd: ls2kbmc: Able to be compiled as a module
>       mfd: ls2kbmc: Capture the reset event of BMC through GPIO
>
>  drivers/char/ipmi/Kconfig   |   2 +-
>  drivers/mfd/Kconfig         |   2 +-
>  drivers/mfd/ls2k-bmc-core.c | 235 ++++++++++++++++++++++++++++++++++----------
>  3 files changed, 184 insertions(+), 55 deletions(-)
> ---
> base-commit: 0e35b9b6ec0ffcc5e23cbdec09f5c622ad532b53
> change-id: 20260626-ls2kbmc-mod-5209193009b2
>
> Best regards,
> --
> Miao Wang <shankerwangmiao@gmail.com>
>
>

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

[Miao Wang]

Hi,

> 2026年7月24日 16:55，Huacai Chen <chenhuacai@kernel.org> 写道：
> 
> Hi, Miao,
> 
> On Fri, Jul 10, 2026 at 1:24 AM Miao Wang via B4 Relay
> <devnull+shankerwangmiao.gmail.com@kernel.org> wrote:
>> 
>> Previously, the driver has been introduced to support the Loongson 2K
>> BMC running on the Loongson Servers, which is essential to prevent
>> the system from hanging when the BMC is being reset and the default
>> efi-framebuffer is being used. However, there are some drawbacks in the
>> driver.
>> 
>> Firstly, the driver tries to read and write to the connected PCI-E host
>> controller registers, assuming that the BMC is connected to LS7A PCI-E
>> host controller. This assumption should be true for real products, but
>> to prevent from accidentally reading and writing to the wrong PCI-E host
>> controller, this driver should be modified to check this before
>> accessing the registers.
>> 
>> Secondly, the driver uses non-exported functions to tell the vt
>> subsystem to redraw the screen, preventing the driver from being
>> compiling as a module. This can be fixed by using the exported
>> functions instead.
> You can replace the redraw function, but I don't think it is necessary
> to make the bmc driver modular.
> 
> BMC core, IPMI and simpledrm display are usually (if not always)
> supposed to work as early as possible.

I believe that it should be the user's decision to choose whether to
compile a module into the kernel or as a module and it would be better
if we can provide the possibilities for the user to choose from.
Additionally, I don't think these modules are supposed to work that
early. The mfd module provide two functions, the display and the ipmi
device. In the aspect of graphical display, without this module, the
user can still see the output during booting on their monitors, since
efifb is working, providing a basic display function. In the aspect of
the ipmi device, I don't think the lack of such device will influence
the boot of the system, since it is a common practice to compile ipmi
device drivers as modules on other architectures. As a result, neither
of the two functions are required to be loaded that early and it is
reasonable to at lease leave the choice to compile it as a module
to the user.

Cheers,

Miao Wang

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

[Huacai Chen]

On Fri, Jul 24, 2026 at 5:28 PM Miao Wang <shankerwangmiao@gmail.com> wrote:
>
> Hi,
>
> > 2026年7月24日 16:55，Huacai Chen <chenhuacai@kernel.org> 写道：
> >
> > Hi, Miao,
> >
> > On Fri, Jul 10, 2026 at 1:24 AM Miao Wang via B4 Relay
> > <devnull+shankerwangmiao.gmail.com@kernel.org> wrote:
> >>
> >> Previously, the driver has been introduced to support the Loongson 2K
> >> BMC running on the Loongson Servers, which is essential to prevent
> >> the system from hanging when the BMC is being reset and the default
> >> efi-framebuffer is being used. However, there are some drawbacks in the
> >> driver.
> >>
> >> Firstly, the driver tries to read and write to the connected PCI-E host
> >> controller registers, assuming that the BMC is connected to LS7A PCI-E
> >> host controller. This assumption should be true for real products, but
> >> to prevent from accidentally reading and writing to the wrong PCI-E host
> >> controller, this driver should be modified to check this before
> >> accessing the registers.
> >>
> >> Secondly, the driver uses non-exported functions to tell the vt
> >> subsystem to redraw the screen, preventing the driver from being
> >> compiling as a module. This can be fixed by using the exported
> >> functions instead.
> > You can replace the redraw function, but I don't think it is necessary
> > to make the bmc driver modular.
> >
> > BMC core, IPMI and simpledrm display are usually (if not always)
> > supposed to work as early as possible.
>
> I believe that it should be the user's decision to choose whether to
> compile a module into the kernel or as a module and it would be better
> if we can provide the possibilities for the user to choose from.
> Additionally, I don't think these modules are supposed to work that
> early. The mfd module provide two functions, the display and the ipmi
> device. In the aspect of graphical display, without this module, the
> user can still see the output during booting on their monitors, since
> efifb is working, providing a basic display function. In the aspect of
> the ipmi device, I don't think the lack of such device will influence
> the boot of the system, since it is a common practice to compile ipmi
> device drivers as modules on other architectures. As a result, neither
> of the two functions are required to be loaded that early and it is
> reasonable to at lease leave the choice to compile it as a module
> to the user.
Flexibility is not always useful, if a config doesn't provide good
effect, then it just increases complexity and makes maintenance more
difficult.

Huacai

>
> Cheers,
>
> Miao Wang
>
>

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

[Miao Wang]

Hi,

> 2026年7月31日 16:07，Huacai Chen <chenhuacai@kernel.org> 写道：
> 
> On Fri, Jul 24, 2026 at 5:28 PM Miao Wang <shankerwangmiao@gmail.com> wrote:
>> 
>> Hi,
>> 
>>> 2026年7月24日 16:55，Huacai Chen <chenhuacai@kernel.org> 写道：
>>> 
>>> Hi, Miao,
>>> 
>>> On Fri, Jul 10, 2026 at 1:24 AM Miao Wang via B4 Relay
>>> <devnull+shankerwangmiao.gmail.com@kernel.org> wrote:
>>>> 
>>>> Previously, the driver has been introduced to support the Loongson 2K
>>>> BMC running on the Loongson Servers, which is essential to prevent
>>>> the system from hanging when the BMC is being reset and the default
>>>> efi-framebuffer is being used. However, there are some drawbacks in the
>>>> driver.
>>>> 
>>>> Firstly, the driver tries to read and write to the connected PCI-E host
>>>> controller registers, assuming that the BMC is connected to LS7A PCI-E
>>>> host controller. This assumption should be true for real products, but
>>>> to prevent from accidentally reading and writing to the wrong PCI-E host
>>>> controller, this driver should be modified to check this before
>>>> accessing the registers.
>>>> 
>>>> Secondly, the driver uses non-exported functions to tell the vt
>>>> subsystem to redraw the screen, preventing the driver from being
>>>> compiling as a module. This can be fixed by using the exported
>>>> functions instead.
>>> You can replace the redraw function, but I don't think it is necessary
>>> to make the bmc driver modular.
>>> 
>>> BMC core, IPMI and simpledrm display are usually (if not always)
>>> supposed to work as early as possible.
>> 
>> I believe that it should be the user's decision to choose whether to
>> compile a module into the kernel or as a module and it would be better
>> if we can provide the possibilities for the user to choose from.
>> Additionally, I don't think these modules are supposed to work that
>> early. The mfd module provide two functions, the display and the ipmi
>> device. In the aspect of graphical display, without this module, the
>> user can still see the output during booting on their monitors, since
>> efifb is working, providing a basic display function. In the aspect of
>> the ipmi device, I don't think the lack of such device will influence
>> the boot of the system, since it is a common practice to compile ipmi
>> device drivers as modules on other architectures. As a result, neither
>> of the two functions are required to be loaded that early and it is
>> reasonable to at lease leave the choice to compile it as a module
>> to the user.
> Flexibility is not always useful, if a config doesn't provide good
> effect, then it just increases complexity and makes maintenance more
> difficult.

I should emphasize that to allow this driver to be a module, there is
no such increase on maintenance. Moreover, not all loongarch machines
are requiring this driver. Especially only a part of the server models
are quipped with this BMC. Comparing with other architectures, the
driver for BMC are normally compiled as a module, such as mgag200 for
iLO from HPE and iDRAC from DELL, hibmc_drm for Kunpeng server from
Huawei. None of these BMC drivers requiring to be compiled into the
kernel. I cannot see there is any reason keeping the driver from being
allowed to be compiled as a module. I also do not think it will bring
any significant maintenance burden. Implementing correct cleanup code
should be necessary instead of burden.

Cheers,

Miao Wang

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

[Huacai Chen]

On Fri, Jul 31, 2026 at 4:24 PM Miao Wang <shankerwangmiao@gmail.com> wrote:
>
> Hi,
>
> > 2026年7月31日 16:07，Huacai Chen <chenhuacai@kernel.org> 写道：
> >
> > On Fri, Jul 24, 2026 at 5:28 PM Miao Wang <shankerwangmiao@gmail.com> wrote:
> >>
> >> Hi,
> >>
> >>> 2026年7月24日 16:55，Huacai Chen <chenhuacai@kernel.org> 写道：
> >>>
> >>> Hi, Miao,
> >>>
> >>> On Fri, Jul 10, 2026 at 1:24 AM Miao Wang via B4 Relay
> >>> <devnull+shankerwangmiao.gmail.com@kernel.org> wrote:
> >>>>
> >>>> Previously, the driver has been introduced to support the Loongson 2K
> >>>> BMC running on the Loongson Servers, which is essential to prevent
> >>>> the system from hanging when the BMC is being reset and the default
> >>>> efi-framebuffer is being used. However, there are some drawbacks in the
> >>>> driver.
> >>>>
> >>>> Firstly, the driver tries to read and write to the connected PCI-E host
> >>>> controller registers, assuming that the BMC is connected to LS7A PCI-E
> >>>> host controller. This assumption should be true for real products, but
> >>>> to prevent from accidentally reading and writing to the wrong PCI-E host
> >>>> controller, this driver should be modified to check this before
> >>>> accessing the registers.
> >>>>
> >>>> Secondly, the driver uses non-exported functions to tell the vt
> >>>> subsystem to redraw the screen, preventing the driver from being
> >>>> compiling as a module. This can be fixed by using the exported
> >>>> functions instead.
> >>> You can replace the redraw function, but I don't think it is necessary
> >>> to make the bmc driver modular.
> >>>
> >>> BMC core, IPMI and simpledrm display are usually (if not always)
> >>> supposed to work as early as possible.
> >>
> >> I believe that it should be the user's decision to choose whether to
> >> compile a module into the kernel or as a module and it would be better
> >> if we can provide the possibilities for the user to choose from.
> >> Additionally, I don't think these modules are supposed to work that
> >> early. The mfd module provide two functions, the display and the ipmi
> >> device. In the aspect of graphical display, without this module, the
> >> user can still see the output during booting on their monitors, since
> >> efifb is working, providing a basic display function. In the aspect of
> >> the ipmi device, I don't think the lack of such device will influence
> >> the boot of the system, since it is a common practice to compile ipmi
> >> device drivers as modules on other architectures. As a result, neither
> >> of the two functions are required to be loaded that early and it is
> >> reasonable to at lease leave the choice to compile it as a module
> >> to the user.
> > Flexibility is not always useful, if a config doesn't provide good
> > effect, then it just increases complexity and makes maintenance more
> > difficult.
>
> I should emphasize that to allow this driver to be a module, there is
> no such increase on maintenance. Moreover, not all loongarch machines
> are requiring this driver. Especially only a part of the server models
> are quipped with this BMC. Comparing with other architectures, the
> driver for BMC are normally compiled as a module, such as mgag200 for
> iLO from HPE and iDRAC from DELL, hibmc_drm for Kunpeng server from
> Huawei. None of these BMC drivers requiring to be compiled into the
> kernel. I cannot see there is any reason keeping the driver from being
> allowed to be compiled as a module. I also do not think it will bring
> any significant maintenance burden. Implementing correct cleanup code
> should be necessary instead of burden.
Can we split into two series, one fix bugs and the others make bmc modular?

Otherwise I don't think we can reach a consensus in the near future.

Huacai

>
> Cheers,
>
> Miao Wang
>
>

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

[Miao Wang]

Hi,

> 2026年8月3日 21:45，Huacai Chen <chenhuacai@kernel.org> 写道：
> 
> On Fri, Jul 31, 2026 at 4:24 PM Miao Wang <shankerwangmiao@gmail.com> wrote:
>> 
>> Hi,
>> 
>>> 2026年7月31日 16:07，Huacai Chen <chenhuacai@kernel.org> 写道：
>>> 
>>> On Fri, Jul 24, 2026 at 5:28 PM Miao Wang <shankerwangmiao@gmail.com> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>>> 2026年7月24日 16:55，Huacai Chen <chenhuacai@kernel.org> 写道：
>>>>> 
>>>>> Hi, Miao,
>>>>> 
>>>>> On Fri, Jul 10, 2026 at 1:24 AM Miao Wang via B4 Relay
>>>>> <devnull+shankerwangmiao.gmail.com@kernel.org> wrote:
>>>>>> 
>>>>>> Previously, the driver has been introduced to support the Loongson 2K
>>>>>> BMC running on the Loongson Servers, which is essential to prevent
>>>>>> the system from hanging when the BMC is being reset and the default
>>>>>> efi-framebuffer is being used. However, there are some drawbacks in the
>>>>>> driver.
>>>>>> 
>>>>>> Firstly, the driver tries to read and write to the connected PCI-E host
>>>>>> controller registers, assuming that the BMC is connected to LS7A PCI-E
>>>>>> host controller. This assumption should be true for real products, but
>>>>>> to prevent from accidentally reading and writing to the wrong PCI-E host
>>>>>> controller, this driver should be modified to check this before
>>>>>> accessing the registers.
>>>>>> 
>>>>>> Secondly, the driver uses non-exported functions to tell the vt
>>>>>> subsystem to redraw the screen, preventing the driver from being
>>>>>> compiling as a module. This can be fixed by using the exported
>>>>>> functions instead.
>>>>> You can replace the redraw function, but I don't think it is necessary
>>>>> to make the bmc driver modular.
>>>>> 
>>>>> BMC core, IPMI and simpledrm display are usually (if not always)
>>>>> supposed to work as early as possible.
>>>> 
>>>> I believe that it should be the user's decision to choose whether to
>>>> compile a module into the kernel or as a module and it would be better
>>>> if we can provide the possibilities for the user to choose from.
>>>> Additionally, I don't think these modules are supposed to work that
>>>> early. The mfd module provide two functions, the display and the ipmi
>>>> device. In the aspect of graphical display, without this module, the
>>>> user can still see the output during booting on their monitors, since
>>>> efifb is working, providing a basic display function. In the aspect of
>>>> the ipmi device, I don't think the lack of such device will influence
>>>> the boot of the system, since it is a common practice to compile ipmi
>>>> device drivers as modules on other architectures. As a result, neither
>>>> of the two functions are required to be loaded that early and it is
>>>> reasonable to at lease leave the choice to compile it as a module
>>>> to the user.
>>> Flexibility is not always useful, if a config doesn't provide good
>>> effect, then it just increases complexity and makes maintenance more
>>> difficult.
>> 
>> I should emphasize that to allow this driver to be a module, there is
>> no such increase on maintenance. Moreover, not all loongarch machines
>> are requiring this driver. Especially only a part of the server models
>> are quipped with this BMC. Comparing with other architectures, the
>> driver for BMC are normally compiled as a module, such as mgag200 for
>> iLO from HPE and iDRAC from DELL, hibmc_drm for Kunpeng server from
>> Huawei. None of these BMC drivers requiring to be compiled into the
>> kernel. I cannot see there is any reason keeping the driver from being
>> allowed to be compiled as a module. I also do not think it will bring
>> any significant maintenance burden. Implementing correct cleanup code
>> should be necessary instead of burden.
> Can we split into two series, one fix bugs and the others make bmc modular?
> 
> Otherwise I don't think we can reach a consensus in the near future.

I accept different opinions on design trade-offs. However, I don't think
you have provided enough excuses to remain this driver as built-in,
since I believe normally in kernel, most non-core drivers are all able
to be compiled as a module. I also provided some examples from devices
with similar functions. As a result, I'll not split this series before
there is indeed a strong reason against allowing this driver to be
compiled as a module or we may have a great benefit if we force this
module to be compiled built-in.

Cheers,

Miao Wang

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

[Huacai Chen]

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

Huacai

>
> Cheers,
>
> Miao Wang

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

[Miao Wang]

Hi, Huacai

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


Sorry for not noticing Binbin's patch which had already been addressing the
same issue. However, I believe the fix in my series has covered more issues
than Binbin's patch and thus should be better in the following aspects:

1. Since the length of the resolution string is short enough, there is no
   need to allocate buffer from the heap. The buffer can be allocated 
   directly on the stack. The most possible resolution string might be
   "video=4294967295x4294967295-4294967295@4294967295M\0", which is 51 bytes,
   and allocating 64 bytes on the stack would be enough.
2. My patch enhances the validation of the mode string. strsep(&ptr, sep) may
   set ptr to NULL if sep is not found, which will cause later strsep()
   invocations returning NULL, which is not expected by kstrtoxx() functions.
3. My patch further guards string manipulation functions by adding the ending
   '\0' to the end of the string buffer, to prevent strsep() from overrunning
   the end of the buffer.
4. My patch also addresses the type of the data. The fields in the struct
   simplefb_platform_data are unsigned and thus kstrtouint() should be used
   instead.
 
If the BMC device is under the control of an attacker somehow and the attacker
is feeding malicious resolution string to the host kernel, then the second and
the third issue will cause memory issues in the host kernel, which might be
dangerous.

I wonder if Binbin is willing to address previous issues in that patch. If
Binbin is willing to do so, then I will be pleased to drop the first patch
from my series. If Binbin agrees, that patch can also be absorbed and replaced
by the first patch in my series.

Cheers,

Miao Wang