在 2025/5/22 下午9:46, Lee Jones 写道:
On Tue, 06 May 2025, Qunqin Zhao wrote:
Loongson Security Engine chip supports RNG, SM2, SM3 and SM4 accelerator
engines. This is the base driver for other specific engine drivers.
Co-developed-by: Yinggang Gu <guyingg...@loongson.cn>
Signed-off-by: Yinggang Gu <guyingg...@loongson.cn>
Signed-off-by: Qunqin Zhao <zhaoqun...@loongson.cn>
Reviewed-by: Huacai Chen <chenhua...@loongson.cn>
---
v8: As explained in the cover letter, moved this driver form MFD to here.
Cleanned up coding style. Added some comments. Divided DMA memory
equally among all engines.
v7: Moved Kconfig entry between MFD_INTEL_M10_BMC_PMCI and MFD_QNAP_MCU.
Renamed se_enable_int_locked() to se_enable_int(), then moved the
lock out of se_disable_int().
"se_send_genl_cmd" ---> "se_send_cmd".
"struct lsse_ch" ---> "struct se_channel".
v6: Replace all "ls6000se" with "loongson"
v5: Registered "ls6000se-rng" device.
v3-v4: None
drivers/mfd/Kconfig | 11 ++
drivers/mfd/Makefile | 2 +
drivers/mfd/loongson-se.c | 235 ++++++++++++++++++++++++++++++++
include/linux/mfd/loongson-se.h | 52 +++++++
4 files changed, 300 insertions(+)
create mode 100644 drivers/mfd/loongson-se.c
create mode 100644 include/linux/mfd/loongson-se.h
General premise seems okay.
Couple of questions and styling / readability issues.
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 22b936310..c2f94b315 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2369,6 +2369,17 @@ config MFD_INTEL_M10_BMC_PMCI
additional drivers must be enabled in order to use the functionality
of the device.
+config MFD_LOONGSON_SE
+ tristate "Loongson Security Engine chip controller driver"
+ depends on LOONGARCH && ACPI
+ select MFD_CORE
+ help
+ The Loongson Security Engine chip supports RNG, SM2, SM3 and
+ SM4 accelerator engines. Each engine have its own DMA buffer
+ provided by the controller. The kernel cannot directly send
+ commands to the engine and must first send them to the controller,
+ which will forward them to the corresponding engine.
+
config MFD_QNAP_MCU
tristate "QNAP microcontroller unit core driver"
depends on SERIAL_DEV_BUS
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 948cbdf42..fc50601ca 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_LOONGSON_SE) += loongson-se.o
diff --git a/drivers/mfd/loongson-se.c b/drivers/mfd/loongson-se.c
new file mode 100644
index 000000000..ce38d8221
--- /dev/null
+++ b/drivers/mfd/loongson-se.c
@@ -0,0 +1,235 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2025 Loongson Technology Corporation Limited */
Author(s)?
Will add
+#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/loongson-se.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+struct loongson_se {
+ void __iomem *base;
+ spinlock_t dev_lock;
+ struct completion cmd_completion;
+
+ void *dmam_base;
+ int dmam_size;
+
+ struct mutex engine_init_lock;
+ struct loongson_se_engine engines[SE_ENGINE_MAX];
+};
+
+struct loongson_se_controller_cmd {
+ u32 command_id;
+ u32 info[7];
+};
+
+static int loongson_se_poll(struct loongson_se *se, u32 int_bit)
+{
+ u32 status;
+ int err;
+
+ spin_lock_irq(&se->dev_lock);
+
+ /* Notify the controller that the engine needs to be started */
+ writel(int_bit, se->base + SE_L2SINT_SET);
Code that is squished together is difficult to read.
'\n'
All '\n' will fix
+ /* Polling until the controller has forwarded the engine command */
+ err = readl_relaxed_poll_timeout_atomic(se->base + SE_L2SINT_STAT,
status,
+ !(status & int_bit), 1, 10000);
How long is that? Why was that number chosen?
In theory, the hardware will clear int_bit within 10ms.
Please define the type, like:
LOONSON_ENGINE_CMD_TIMEOUT_MS 10000
Will do this. But this should be us.
LOONSON_ENGINE_CMD_TIMEOUT_US 10000
... or whatever it is.
+ spin_unlock_irq(&se->dev_lock);
+
+ return err;
+}
+
+static int loongson_se_send_controller_cmd(struct loongson_se *se,
+ struct loongson_se_controller_cmd
*cmd)
+{
+ u32 *send_cmd = (u32 *)cmd;
+ int err, i;
+
+ for (i = 0; i < SE_SEND_CMD_REG_LEN; i++)
+ writel(send_cmd[i], se->base + SE_SEND_CMD_REG + i * 4);
Is there any reason not to use regmap?
I'm not familiar with regmap, readl/writel is fine for me.
If regmap is necessary, I will try using it.
+ err = loongson_se_poll(se, SE_INT_CONTROLLER);
+ if (err)
+ return err;
+
+ return wait_for_completion_interruptible(&se->cmd_completion);
+}
+
+int loongson_se_send_engine_cmd(struct loongson_se_engine *engine)
+{
+ /* After engine initialization, the controller already knows
+ * where to obtain engine commands from. Now all we need to
+ * do is notify the controller that the engine needs to be started.
+ */
This is not a proper multi-line comment as per Coding Style.
Will fix
+ int err = loongson_se_poll(engine->se, BIT(engine->id));
+
+ if (err)
+ return err;
+
+ return wait_for_completion_interruptible(&engine->completion);
+}
+EXPORT_SYMBOL_GPL(loongson_se_send_engine_cmd);
+
+struct loongson_se_engine *loongson_se_init_engine(struct device *dev, int id)
What calls this? Whose 'dev' is that?
Child device driver would call this, like "loongson-tpm" and "loongson-rng".
"dev" is the device we probed in "loongson_se_probe".
+{
+ struct loongson_se *se = dev_get_drvdata(dev);
+ struct loongson_se_engine *engine = &se->engines[id];
+ struct loongson_se_controller_cmd cmd;
+
+ engine->se = se;
+ engine->id = id;
+ init_completion(&engine->completion);
+
+ /* Divide DMA memory equally among all engines */
+ engine->buffer_size = se->dmam_size / SE_ENGINE_MAX;
+ engine->buffer_off = (se->dmam_size / SE_ENGINE_MAX) * id;
+ engine->data_buffer = se->dmam_base + engine->buffer_off;
+
+ /*
+ * There has no engine0, use its data buffer as command buffer for other
+ * engines. The DMA memory size is obtained from the ACPI table, which
+ * ensures that the data buffer size of engine0 is larger than the
+ * command buffer size of all engines.
+ */
+ engine->command = se->dmam_base + id * (2 * SE_ENGINE_CMD_SIZE);
Why 2?
One for engine->command, another for engine->command_ret.
+ engine->command_ret = engine->command + SE_ENGINE_CMD_SIZE;
+
+ mutex_lock(&se->engine_init_lock);
....
+ while (nr_irq) {
+ irq = platform_get_irq(pdev, --nr_irq);
Do the decrement separately at the end of the statement, not hidden here.
Or, probably better still, use a for() loop.
Will use a for() loop
+ err = devm_request_irq(dev, irq, se_irq_handler, 0,
"loongson-se", se);
+ if (err)
+ dev_err(dev, "failed to request irq: %d\n", irq);
IRQ
Will fix
+ }
+
+ err = loongson_se_init(se, paddr, se->dmam_size);
+ if (err)
+ return err;
+
+ return devm_mfd_add_devices(dev, 0, engines, ARRAY_SIZE(engines), NULL,
0, NULL);
Why 0?
Next revision:
return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, engines,
ARRAY_SIZE(engines), NULL, 0, NULL);
+}
+
+static const struct acpi_device_id loongson_se_acpi_match[] = {
+ {"LOON0011", 0},
There should be spaces after the '{' and before the '}'.
Sorry, Will fix
+ {}
+};
+MODULE_DEVICE_TABLE(acpi, loongson_se_acpi_match);
+
+static struct platform_driver loongson_se_driver = {
+ .probe = loongson_se_probe,
+ .driver = {
+ .name = "loongson-se",
+ .acpi_match_table = loongson_se_acpi_match,
+ },
+};
+module_platform_driver(loongson_se_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Yinggang Gu <guyingg...@loongson.cn>");
+MODULE_AUTHOR("Qunqin Zhao <zhaoqun...@loongson.cn>");
+MODULE_DESCRIPTION("Loongson Security Engine chip controller driver");
diff --git a/include/linux/mfd/loongson-se.h b/include/linux/mfd/loongson-se.h
new file mode 100644
index 000000000..f962d6143
--- /dev/null
+++ b/include/linux/mfd/loongson-se.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (C) 2025 Loongson Technology Corporation Limited */
+
+#ifndef __LOONGSON_SE_H__
+#define __LOONGSON_SE_H__
__MFD_*
Will fix
+#define SE_SEND_CMD_REG 0x0
+#define SE_SEND_CMD_REG_LEN 0x8
+/* controller command id */
Uppercase char to start comments.
"ID"
Will fix
Thanks,
Qunqin.
