Re: [PATCH 2/2] soc: qcom: Add Qualcomm Ramp Controller driver
From: AngeloGioacchino Del Regno
Date: Fri Nov 04 2022 - 10:07:10 EST
Il 04/11/22 15:04, Krzysztof Kozlowski ha scritto:
On 04/11/2022 09:35, AngeloGioacchino Del Regno wrote:
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxxx>
The Ramp Controller is used to program the sequence ID for pulse
swallowing, enable sequence and linking sequence IDs for the CPU
cores on some Qualcomm SoCs.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxxx>
---
drivers/soc/qcom/Kconfig | 9 +
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/ramp_controller.c | 330 +++++++++++++++++++++++++++++
3 files changed, 340 insertions(+)
create mode 100644 drivers/soc/qcom/ramp_controller.c
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 024e420f1bb7..1e681f98bad4 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -95,6 +95,15 @@ config QCOM_QMI_HELPERS
tristate
depends on NET
+config QCOM_RAMP_CTRL
+ tristate "Qualcomm Ramp Controller driver"
+ depends on ARCH_QCOM
I propose:
depends on ARCH_QCOM && ARM || COMPILE_TEST
I don't think it is used on ARM64 SoCs, so let's make life of distros
easier.
Agreed.
+ help
+ The Ramp Controller is used to program the sequence ID for pulse
+ swallowing, enable sequence and linking sequence IDs for the
+ CPU cores on some Qualcomm SoCs.
+ Say y here to enable support for the ramp controller.
+
config QCOM_RMTFS_MEM
tristate "Qualcomm Remote Filesystem memory driver"
depends on ARCH_QCOM
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index d66604aff2b0..6e02333c4080 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_QCOM_OCMEM) += ocmem.o
obj-$(CONFIG_QCOM_PDR_HELPERS) += pdr_interface.o
obj-$(CONFIG_QCOM_QMI_HELPERS) += qmi_helpers.o
qmi_helpers-y += qmi_encdec.o qmi_interface.o
+obj-$(CONFIG_QCOM_RAMP_CTRL) += ramp_controller.o
obj-$(CONFIG_QCOM_RMTFS_MEM) += rmtfs_mem.o
obj-$(CONFIG_QCOM_RPMH) += qcom_rpmh.o
qcom_rpmh-y += rpmh-rsc.o
diff --git a/drivers/soc/qcom/ramp_controller.c b/drivers/soc/qcom/ramp_controller.c
new file mode 100644
index 000000000000..e28679b545d1
--- /dev/null
+++ b/drivers/soc/qcom/ramp_controller.c
@@ -0,0 +1,330 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Qualcomm Ramp Controller driver
+ * Copyright (c) 2022, AngeloGioacchino Del Regno
+ * <angelogioacchino.delregno@xxxxxxxxxxxxx>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+#define RC_UPDATE_EN BIT(0)
+#define RC_ROOT_EN BIT(1)
+
+#define RC_REG_CFG_UPDATE 0x60
+ #define RC_CFG_UPDATE_EN BIT(8)
+ #define RC_CFG_ACK GENMASK(31, 16)
Drop spaces before #define
+
+#define RC_DCVS_CFG_SID 2
+#define RC_LINK_SID 3
+#define RC_LMH_SID 6
+#define RC_DFS_SID 14
+
+#define RC_UPDATE_TIMEOUT_US 500
+
+/**
+ * struct qcom_ramp_controller_desc - SoC specific parameters
+ * @cfg_dfs_sid: Dynamic Frequency Scaling SID configuration
+ * @cfg_link_sid: Link SID configuration
+ * @cfg_lmh_sid: Limits Management hardware SID configuration
+ * @cfg_ramp_pre_en: Ramp Controller pre-enable sequence
+ * @cfg_ramp_en: Ramp Controller enable sequence
+ * @cfg_ramp_post_en: Ramp Controller post-enable sequence
+ * @cfg_ramp_dis: Ramp Controller disable sequence
+ * @cmd_reg: Command register offset
+ * @num_dfs_sids: Number of DFS SIDs (max 8)
+ * @num_link_sids: Number of Link SIDs (max 3)
+ * @num_lmh_sids: Number of LMh SIDs (max 8)
+ */
+struct qcom_ramp_controller_desc {
+ struct reg_sequence *cfg_dfs_sid;
I didn't check much, but can these be pointers to const?
Yeah, const is the way. Will do.
+ struct reg_sequence *cfg_link_sid;
+ struct reg_sequence *cfg_lmh_sid;
+ struct reg_sequence *cfg_ramp_pre_en;
+ struct reg_sequence *cfg_ramp_en;
+ struct reg_sequence *cfg_ramp_post_en;
+ struct reg_sequence *cfg_ramp_dis;
+ u8 cmd_reg;
+ u8 num_dfs_sids;
+ u8 num_link_sids;
+ u8 num_lmh_sids;
+};
+
(...)
+
+static struct platform_driver qcom_ramp_controller_driver = {
+ .driver = {
+ .name = "qcom-ramp-controller",
+ .of_match_table = qcom_ramp_controller_match_table,
+ .suppress_bind_attrs = true,
+ },
+ .probe = qcom_ramp_controller_probe,
+ .remove = qcom_ramp_controller_remove,
+};
+
+static int __init qcom_ramp_controller_init(void)
+{
+ return platform_driver_register(&qcom_ramp_controller_driver);
+}
+arch_initcall(qcom_ramp_controller_init);
Does it really have to be arch initcall? Cannot be module platform driver?
Cannot be platform driver. This has to initialize as early as possible, or
booting will be unstable in some cases (big cluster enabled).
Cheers!
Angelo