| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250624030154.GB10415@nxa18884-linux>
Date: Tue, 24 Jun 2025 11:01:54 +0800
From: Peng Fan <peng.fan@....nxp.com>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
matthias.bgg@...il.com, sudeep.holla@....com,
cristian.marussi@....com, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, arm-scmi@...r.kernel.org,
kernel@...labora.com
Subject: Re: [PATCH v1 2/2] firmware: arm_scmi: Add MediaTek TinySYS SCMI
Protocol support
On Mon, Jun 23, 2025 at 02:01:36PM +0200, AngeloGioacchino Del Regno wrote:
>Add a driver for the SCMI protocol extensions for MediaTek TinySYS.
>This is used to communicate with various remote processors in some
>MediaTek SoCs, which mainly handle power management related tasks.
>
>Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
>---
> drivers/firmware/arm_scmi/Kconfig | 1 +
> drivers/firmware/arm_scmi/Makefile | 1 +
> .../arm_scmi/vendors/mediatek/Kconfig | 16 +
> .../arm_scmi/vendors/mediatek/Makefile | 2 +
> .../arm_scmi/vendors/mediatek/mtk-tinysys.c | 344 ++++++++++++++++++
> include/linux/scmi_mtk_protocol.h | 62 ++++
> 6 files changed, 426 insertions(+)
> create mode 100644 drivers/firmware/arm_scmi/vendors/mediatek/Kconfig
> create mode 100644 drivers/firmware/arm_scmi/vendors/mediatek/Makefile
> create mode 100644 drivers/firmware/arm_scmi/vendors/mediatek/mtk-tinysys.c
> create mode 100644 include/linux/scmi_mtk_protocol.h
>
>diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
>index e3fb36825978..baadf4f7fef6 100644
>--- a/drivers/firmware/arm_scmi/Kconfig
>+++ b/drivers/firmware/arm_scmi/Kconfig
>@@ -84,6 +84,7 @@ config ARM_SCMI_QUIRKS
>
> source "drivers/firmware/arm_scmi/transports/Kconfig"
> source "drivers/firmware/arm_scmi/vendors/imx/Kconfig"
>+source "drivers/firmware/arm_scmi/vendors/mediatek/Kconfig"
>
> endif #ARM_SCMI_PROTOCOL
>
>diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
>index 780cd62b2f78..d1b4ec16b8bc 100644
>--- a/drivers/firmware/arm_scmi/Makefile
>+++ b/drivers/firmware/arm_scmi/Makefile
>@@ -13,6 +13,7 @@ scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
>
> obj-$(CONFIG_ARM_SCMI_PROTOCOL) += transports/
> obj-$(CONFIG_ARM_SCMI_PROTOCOL) += vendors/imx/
>+obj-$(CONFIG_ARM_SCMI_PROTOCOL) += vendors/mediatek/
>
> obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
> obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o
>diff --git a/drivers/firmware/arm_scmi/vendors/mediatek/Kconfig b/drivers/firmware/arm_scmi/vendors/mediatek/Kconfig
>new file mode 100644
>index 000000000000..8facdcd3819f
>--- /dev/null
>+++ b/drivers/firmware/arm_scmi/vendors/mediatek/Kconfig
>@@ -0,0 +1,16 @@
>+# SPDX-License-Identifier: GPL-2.0-only
>+menu "ARM SCMI MediaTek Vendor Protocols"
>+
>+config MTK_SCMI_TINYSYS
>+ tristate "MediaTek SCMI TinySYS Extension"
>+ depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF)
Should dependency to ARCH_MEDIATEK be added, as below
ARM_SCMI_PROTOCOL & ARCH_MEDIATEK ?
>+ default y if ARCH_MEDIATEK
>+ help
>+ This enables communication with the MediaTek TinySYS MCU
>+ to control the power status of various SoC sub-devices
>+ other than passing other messages for initialization.
>+
>+ To compile this driver as a module, choose M here: the
>+ module will be called mtk-tinysys.
>+
>+endmenu
>diff --git a/drivers/firmware/arm_scmi/vendors/mediatek/Makefile b/drivers/firmware/arm_scmi/vendors/mediatek/Makefile
>new file mode 100644
>index 000000000000..dc1ff63c3b69
>--- /dev/null
>+++ b/drivers/firmware/arm_scmi/vendors/mediatek/Makefile
>@@ -0,0 +1,2 @@
>+# SPDX-License-Identifier: GPL-2.0-only
>+obj-$(CONFIG_MTK_SCMI_TINYSYS) += mtk-tinysys.o
>diff --git a/drivers/firmware/arm_scmi/vendors/mediatek/mtk-tinysys.c b/drivers/firmware/arm_scmi/vendors/mediatek/mtk-tinysys.c
>new file mode 100644
>index 000000000000..baeb36493952
>--- /dev/null
>+++ b/drivers/firmware/arm_scmi/vendors/mediatek/mtk-tinysys.c
>@@ -0,0 +1,344 @@
>+// SPDX-License-Identifier: GPL-2.0
>+/*
>+ * System Control and Management Interface (SCMI) MediaTek TinySYS Protocol
>+ *
>+ * Copyright (c) 2021 MediaTek Inc.
>+ * Copyright (c) 2025 Collabora Ltd
>+ * AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
>+ */
>+
>+#define pr_fmt(fmt) "SCMI Notifications TinySYS - " fmt
>+
>+#include <linux/bits.h>
>+#include <linux/io.h>
>+#include <linux/module.h>
>+#include <linux/of.h>
>+#include <linux/platform_device.h>
This header is not needed, since no user here.
I just see i.MX also includes this header, but could be dropped.
>+#include <linux/scmi_protocol.h>
>+#include <linux/scmi_mtk_protocol.h>
>+
>+#include "../../protocols.h"
>+#include "../../notify.h"
>+
>+#define SCMI_MTK_CMD_SSPM_QUERY_ALIVE 0xdead
>+
>+enum scmi_mtk_tinysys_protocol_cmd {
>+ MTK_TINYSYS_COMMON_SET = 0x3,
>+ MTK_TINYSYS_COMMON_GET = 0x4,
>+ MTK_TINYSYS_POWER_STATE_NOTIFY = 0x5,
>+ MTK_TINYSYS_SLBC_CTRL = 0x6,
>+};
>+
>+struct scmi_mtk_tinysys_common_get_payld {
>+ __le32 rsvd;
>+ __le32 param[SCMI_MTK_MSG_COMMON_REPLY_BYTES];
>+};
>+
>+struct scmi_mtk_tinysys_common_set_payld {
>+ __le32 rsvd;
>+ __le32 ctrl_id;
>+ __le32 param[SCMI_MTK_MSG_COMMON_PARAM_BYTES];
>+};
>+
>+struct scmi_mtk_tinysys_slbc_payld {
>+ __le32 rsvd;
>+ __le32 cmd;
>+ __le32 arg[SCMI_MTK_MSG_SLBC_PARAM_BYTES];
>+};
>+
>+struct scmi_mtk_tinysys_pwrst_notify {
>+ __le32 rsvd;
Not sure why all upper structure has 'rsvd', meaning reserved?
>+ __le32 fid;
>+ __le32 enable;
>+};
>+
>+struct scmi_mtk_tinysys_notify_payld {
>+ __le32 fid;
>+ __le32 param[SCMI_MTK_MSG_NOTIF_ST_BYTES];
>+};
>+
>+struct scmi_mtk_tinysys_protocol_attributes {
>+ __le32 attributes;
>+};
>+
>+struct scmi_mtk_tinysys_info {
>+ int num_domains;
>+};
>+
>+static int scmi_mtk_tinysys_attributes_get(const struct scmi_protocol_handle *ph,
>+ struct scmi_mtk_tinysys_info *tinfo)
>+{
>+ struct scmi_mtk_tinysys_protocol_attributes *attr;
>+ struct scmi_xfer *t;
>+ int ret;
>+
>+ ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0, sizeof(*attr), &t);
>+ if (ret)
>+ return ret;
>+
>+ attr = t->rx.buf;
>+
>+ ret = ph->xops->do_xfer(ph, t);
>+ if (!ret) {
>+ attr->attributes = get_unaligned_le32(t->rx.buf);
>+ tinfo->num_domains = attr->attributes;
Not sure the spec use whole 32bits for num_domains, if not, better
use le32_get_bits to the expect fields
>+ }
>+
>+ ph->xops->xfer_put(ph, t);
>+
>+ return ret;
>+}
>+
>+static int scmi_mtk_tinysys_get_num_sources(const struct scmi_protocol_handle *ph)
>+{
>+ struct scmi_mtk_tinysys_info *tinfo = ph->get_priv(ph);
>+
>+ if (!tinfo)
>+ return -EINVAL;
>+
>+ return tinfo->num_domains;
>+}
>+
>+static int scmi_mtk_tinysys_set_notify_enabled(const struct scmi_protocol_handle *ph,
>+ u8 evt_id, u32 src_id, bool enable)
>+{
>+ struct scmi_mtk_tinysys_pwrst_notify *pwrst_notify;
>+ struct scmi_xfer *t;
>+ int ret;
>+
>+ /* There's only one possible event for now */
>+ if (evt_id != 0)
Use 'evt_id != SCMI_EVENT_MTK_TINYSYS_NOTIFIER' ?
>+ return -EINVAL;
>+
...
>+#endif
>--
>2.49.0
>
Regards,
Peng
Powered by blists - more mailing lists