[v2] LoongArch: Add Loongson-2K0500 BMC support

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

Posted by Binbin Zhou 7 months, 1 week ago

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

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

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

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

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

Posted by Huacai Chen 7 months ago

Hi, Binbin,

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

Huacai

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

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

Posted by Lee Jones 7 months ago

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

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

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

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

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

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

No changes since 2024?

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

"Mainline"

Typically we just do:

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

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

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

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

I2C

> + * adapt to the resolution.

Remove the line break here.

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

BMC

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

PCI

BAR's (maybe?)

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

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

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

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



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

There is no need to pre-initialise this.

> +	resource_size_t base;
> +	struct simplefb_platform_data pd;

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

"Failed to removed firmware framebuffers"

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

Remove this line.

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

-- 
Lee Jones [李琼斯]

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

Posted by Binbin Zhou 7 months ago

Hi Lee:

Thanks for your detailed review.

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

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

How about:

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

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

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

-- 
Thanks.
Binbin

[PATCH v2 1/3] mfd: ls2kbmc: Introduce Loongson-2K BMC MFD Core driver
[PATCH v2 2/3] ipmi: Add Loongson-2K BMC support
[PATCH v2 3/3] mfd: ls2kbmc: Add Loongson-2K BMC reset function support