Re: [PATCH 2/2] soc: qcom: Add Qualcomm Ramp Controller driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux