Re: [PATCH 3/3] clk: qcom: Add SM6115 LPASSCC

On Sat, 26 Aug 2023 at 12:23, Krzysztof Kozlowski
[off-list ref] wrote:

On 25/08/2023 20:13, Konrad Dybcio wrote:

quoted

SM6115 (and its derivatives or similar SoCs) have a LPASS clock
controller block which provides audio-related resets.

Add the required code to support them.

Signed-off-by: Konrad Dybcio <redacted>
---
 drivers/clk/qcom/Kconfig          |  9 +++++
 drivers/clk/qcom/Makefile         |  1 +
 drivers/clk/qcom/lpasscc-sm6115.c | 84 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+)

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index bd9bfb11b328..df9cf112e4b6 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig

@@ -1001,6 +1001,15 @@ config SM_GPUCC_8550
        Say Y if you want to support graphics controller devices and
        functionality such as 3D graphics.

+config SM_LPASSCC_6115
+     tristate "SM6115 Low Power Audio Subsystem (LPASS) Clock Controller"
+     depends on ARM64 || COMPILE_TEST
+     select SM_GCC_6115
+     help
+       Support for the LPASS clock controller on SM6115 devices.
+       Say Y if you want to toggle LPASS-adjacent resets within
+       this clock controller to reset the LPASS subsystem.
+
 config SM_TCSRCC_8550
      tristate "SM8550 TCSR Clock Controller"
      depends on ARM64 || COMPILE_TEST

diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 4790c8cca426..61e3c72fe954 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile

@@ -128,6 +128,7 @@ obj-$(CONFIG_SM_GPUCC_8250) += gpucc-sm8250.o
 obj-$(CONFIG_SM_GPUCC_8350) += gpucc-sm8350.o
 obj-$(CONFIG_SM_GPUCC_8450) += gpucc-sm8450.o
 obj-$(CONFIG_SM_GPUCC_8550) += gpucc-sm8550.o
+obj-$(CONFIG_SM_LPASSCC_6115) += lpasscc-sm6115.o
 obj-$(CONFIG_SM_TCSRCC_8550) += tcsrcc-sm8550.o
 obj-$(CONFIG_SM_VIDEOCC_8150) += videocc-sm8150.o
 obj-$(CONFIG_SM_VIDEOCC_8250) += videocc-sm8250.o

diff --git a/drivers/clk/qcom/lpasscc-sm6115.c b/drivers/clk/qcom/lpasscc-sm6115.c
new file mode 100644
index 000000000000..6aa19e16c53b
--- /dev/null
+++ b/drivers/clk/qcom/lpasscc-sm6115.c

@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022, 2023 Linaro Limited
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/qcom,sm6115-lpasscc.h>
+
+#include "common.h"
+#include "reset.h"
+
+static const struct qcom_reset_map lpass_audiocc_sm6115_resets[] = {
+     [LPASS_AUDIO_SWR_RX_CGCR] =  { .reg = 0x98, .bit = 1, .udelay = 500 },
+};
+
+static struct regmap_config lpass_audiocc_sm6115_regmap_config = {
+     .reg_bits = 32,
+     .reg_stride = 4,
+     .val_bits = 32,
+     .name = "lpass-audio-csr",
+     .max_register = 0x1000,
+};
+
+static const struct qcom_cc_desc lpass_audiocc_sm6115_reset_desc = {
+     .config = &lpass_audiocc_sm6115_regmap_config,
+     .resets = lpass_audiocc_sm6115_resets,
+     .num_resets = ARRAY_SIZE(lpass_audiocc_sm6115_resets),
+};
+
+static const struct qcom_reset_map lpasscc_sm6115_resets[] = {
+     [LPASS_SWR_TX_CONFIG_CGCR] = { .reg = 0x100, .bit = 1, .udelay = 500 },
+};
+
+static struct regmap_config lpasscc_sm6115_regmap_config = {
+     .reg_bits = 32,
+     .reg_stride = 4,
+     .val_bits = 32,
+     .name = "lpass-tcsr",
+     .max_register = 0x1000,
+};
+
+static const struct qcom_cc_desc lpasscc_sm6115_reset_desc = {
+     .config = &lpasscc_sm6115_regmap_config,
+     .resets = lpasscc_sm6115_resets,
+     .num_resets = ARRAY_SIZE(lpasscc_sm6115_resets),
+};
+
+static const struct of_device_id lpasscc_sm6115_match_table[] = {
+     {
+             .compatible = "qcom,sm6115-lpassaudiocc",
+             .data = &lpass_audiocc_sm6115_reset_desc,
+     }, {
+             .compatible = "qcom,sm6115-lpasscc",
+             .data = &lpasscc_sm6115_reset_desc,
+     },
+     { },
+};
+MODULE_DEVICE_TABLE(of, lpasscc_sm6115_match_table);

Everything here is almost the same as sc8280xp one, so this should be
added to sc8280xp. You cut some boilerplate and additional driver.

We have been there. It quickly becomes a nightmare to maintain.
Consider dispcc-sm8250.c

But I agree with you, this code looks too similar. If we expect more
similar lpasscc drivers, which provide no clocks, just several resets,
maybe we can create a common generic wrapper and make resets lists
corresponding driver data?

-- 
With best wishes
Dmitry