* [PATCH 0/5] clk: qcom: sc8280xp: add lpasscc reset control
@ 2023-05-18 11:37 Srinivas Kandagatla
  2023-05-18 11:37 ` [PATCH 1/5] dt-bindings: clock: Add YAML schemas for LPASSCC and reset on SC8280XP Srinivas Kandagatla
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2023-05-18 11:37 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt
  Cc: johan+linaro, agross, konrad.dybcio, mturquette, sboyd, conor+dt,
	linux-arm-msm, linux-clk, devicetree, linux-kernel,
	Srinivas Kandagatla
On SC8280XP, LPASS IP is controlled by q6dsp, however the reset lines
required by some of the IPs like Soundwire still need to be programmed from
Apps processor. This patchset adds support to reset controller on LPASS
CC and LPASS AudioCC.
Tested on X13s.
Thanks,
Srini
Srinivas Kandagatla (5):
  dt-bindings: clock: Add YAML schemas for LPASSCC and reset on SC8280XP
  dt-bindings: clock: Add YAML schemas for LPASS AUDIOCC and reset on
    SC8280XP
  clk: qcom: Add lpass clock controller driver for SC8280XP
  clk: qcom: Add lpass audio clock controller driver for SC8280XP
  arm64: dts: qcom: sc8280xp: add resets for soundwire controllers
 .../bindings/clock/qcom,sc8280xp-lpasscc.yaml | 68 ++++++++++++++
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi        | 19 ++++
 drivers/clk/qcom/Kconfig                      |  8 ++
 drivers/clk/qcom/Makefile                     |  1 +
 drivers/clk/qcom/lpasscc-sc8280xp.c           | 94 +++++++++++++++++++
 .../dt-bindings/clock/qcom,lpasscc-sc8280xp.h | 17 ++++
 6 files changed, 207 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
 create mode 100644 drivers/clk/qcom/lpasscc-sc8280xp.c
 create mode 100644 include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
-- 
2.25.1
^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH 1/5] dt-bindings: clock: Add YAML schemas for LPASSCC and reset on SC8280XP
  2023-05-18 11:37 [PATCH 0/5] clk: qcom: sc8280xp: add lpasscc reset control Srinivas Kandagatla
@ 2023-05-18 11:37 ` Srinivas Kandagatla
  2023-05-22  8:21   ` Johan Hovold
  2023-05-18 11:37 ` [PATCH 2/5] dt-bindings: clock: Add YAML schemas for LPASS AUDIOCC " Srinivas Kandagatla
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Srinivas Kandagatla @ 2023-05-18 11:37 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt
  Cc: johan+linaro, agross, konrad.dybcio, mturquette, sboyd, conor+dt,
	linux-arm-msm, linux-clk, devicetree, linux-kernel,
	Srinivas Kandagatla
The LPASS(Low Power Audio Subsystem) clock provider provides reset
controller support when is driven by the Q6DSP.
This patch adds support for those resets and adds IDs for clients
to request the reset.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 .../bindings/clock/qcom,sc8280xp-lpasscc.yaml | 57 +++++++++++++++++++
 .../dt-bindings/clock/qcom,lpasscc-sc8280xp.h | 12 ++++
 2 files changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
 create mode 100644 include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
diff --git a/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
new file mode 100644
index 000000000000..7c30614a0af9
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/qcom,sc8280xp-lpasscc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm LPASS Core & Audio Clock Controller on SC8280XP
+
+maintainers:
+  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
+
+description: |
+  Qualcomm LPASS core and audio clock control module provides the clocks,
+  reset and power domains on SC8280XP.
+
+  See also::
+    include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
+
+properties:
+  reg: true
+
+  compatible:
+    enum:
+      - qcom,sc8280xp-lpasscc
+
+  '#reset-cells':
+    const: 1
+
+  '#clock-cells':
+    const: 1
+
+  qcom,adsp-pil-mode:
+    description:
+      Indicates if the LPASS would be brought out of reset using
+      peripheral loader.
+    type: boolean
+
+required:
+  - compatible
+  - reg
+  - qcom,adsp-pil-mode
+  - '#reset-cells'
+  - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
+    lpasscc: clock-controller@3900000 {
+      compatible = "qcom,sc8280xp-lpasscc";
+      reg = <0x033e0000 0x12000>;
+      #reset-cells = <1>;
+      #clock-cells = <1>;
+      qcom,adsp-pil-mode;
+    };
+...
diff --git a/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h b/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
new file mode 100644
index 000000000000..df800ea2741c
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2023, Linaro Ltd.
+ */
+
+#ifndef _DT_BINDINGS_CLK_QCOM_LPASSCC_SC8280XP_H
+#define _DT_BINDINGS_CLK_QCOM_LPASSCC_SC8280XP_H
+
+/* LPASS TCSR */
+#define LPASS_AUDIO_SWR_TX_CGCR				0
+
+#endif
-- 
2.25.1
^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 2/5] dt-bindings: clock: Add YAML schemas for LPASS AUDIOCC and reset on SC8280XP
  2023-05-18 11:37 [PATCH 0/5] clk: qcom: sc8280xp: add lpasscc reset control Srinivas Kandagatla
  2023-05-18 11:37 ` [PATCH 1/5] dt-bindings: clock: Add YAML schemas for LPASSCC and reset on SC8280XP Srinivas Kandagatla
@ 2023-05-18 11:37 ` Srinivas Kandagatla
  2023-05-22  8:24   ` Johan Hovold
  2023-05-18 11:37 ` [PATCH 3/5] clk: qcom: Add lpass clock controller driver for SC8280XP Srinivas Kandagatla
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Srinivas Kandagatla @ 2023-05-18 11:37 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt
  Cc: johan+linaro, agross, konrad.dybcio, mturquette, sboyd, conor+dt,
	linux-arm-msm, linux-clk, devicetree, linux-kernel,
	Srinivas Kandagatla
The LPASS(Low Power Audio Subsystem) Audio clock provider provides reset
controller support when is driven by the Q6DSP.
This patch adds support for those resets and adds IDs for clients
to request the reset.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 .../bindings/clock/qcom,sc8280xp-lpasscc.yaml         | 11 +++++++++++
 include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h     |  5 +++++
 2 files changed, 16 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
index 7c30614a0af9..394833819ba3 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
@@ -22,6 +22,7 @@ properties:
   compatible:
     enum:
       - qcom,sc8280xp-lpasscc
+      - qcom,sc8280xp-lpassaudiocc
 
   '#reset-cells':
     const: 1
@@ -45,6 +46,16 @@ required:
 additionalProperties: false
 
 examples:
+  - |
+    #include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
+    lpass_audiocc: clock-controller@3300000 {
+      compatible = "qcom,sc8280xp-lpassaudiocc";
+      reg = <0x32a9000 0x1000>;
+      #reset-cells = <1>;
+      #clock-cells = <1>;
+      qcom,adsp-pil-mode;
+    };
+
   - |
     #include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
     lpasscc: clock-controller@3900000 {
diff --git a/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h b/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
index df800ea2741c..d190d57fc81a 100644
--- a/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
+++ b/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
@@ -6,6 +6,11 @@
 #ifndef _DT_BINDINGS_CLK_QCOM_LPASSCC_SC8280XP_H
 #define _DT_BINDINGS_CLK_QCOM_LPASSCC_SC8280XP_H
 
+/* LPASS AUDIO CC CSR */
+#define LPASS_AUDIO_SWR_RX_CGCR				0
+#define LPASS_AUDIO_SWR_WSA_CGCR			1
+#define LPASS_AUDIO_SWR_WSA2_CGCR			2
+
 /* LPASS TCSR */
 #define LPASS_AUDIO_SWR_TX_CGCR				0
 
-- 
2.25.1
^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 3/5] clk: qcom: Add lpass clock controller driver for SC8280XP
  2023-05-18 11:37 [PATCH 0/5] clk: qcom: sc8280xp: add lpasscc reset control Srinivas Kandagatla
  2023-05-18 11:37 ` [PATCH 1/5] dt-bindings: clock: Add YAML schemas for LPASSCC and reset on SC8280XP Srinivas Kandagatla
  2023-05-18 11:37 ` [PATCH 2/5] dt-bindings: clock: Add YAML schemas for LPASS AUDIOCC " Srinivas Kandagatla
@ 2023-05-18 11:37 ` Srinivas Kandagatla
  2023-05-22  8:31   ` Johan Hovold
  2023-05-22  8:45   ` Johan Hovold
  2023-05-18 11:37 ` [PATCH 4/5] clk: qcom: Add lpass audio " Srinivas Kandagatla
  2023-05-18 11:38 ` [PATCH 5/5] arm64: dts: qcom: sc8280xp: add resets for soundwire controllers Srinivas Kandagatla
  4 siblings, 2 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2023-05-18 11:37 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt
  Cc: johan+linaro, agross, konrad.dybcio, mturquette, sboyd, conor+dt,
	linux-arm-msm, linux-clk, devicetree, linux-kernel,
	Srinivas Kandagatla
Add support for the lpass clock controller found on SC8280XP based devices.
This would allow lpass peripheral loader drivers to control the clocks and
bring the subsystems out of reset.
Currently this patch only supports resets as the Q6DSP is in control of
LPASS IP which manages most of the clocks via Q6PRM service on GPR rpmsg
channel.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/clk/qcom/Kconfig            |  8 ++++
 drivers/clk/qcom/Makefile           |  1 +
 drivers/clk/qcom/lpasscc-sc8280xp.c | 71 +++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+)
 create mode 100644 drivers/clk/qcom/lpasscc-sc8280xp.c
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 12be3e2371b3..8188f4dedf40 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -514,6 +514,14 @@ config SC_GPUCC_8280XP
 	  Say Y if you want to support graphics controller devices and
 	  functionality such as 3D graphics.
 
+config SC_LPASSCC_8280XP
+	tristate "SC8280 Low Power Audio Subsystem (LPASS) Clock Controller"
+	select SC_GCC_8280XP
+	help
+	  Support for the LPASS clock controller on SC8280XP devices.
+	  Say Y if you want to use the LPASS branch clocks of the LPASS clock
+	  controller to reset the LPASS subsystem.
+
 config SC_LPASSCC_7280
 	tristate "SC7280 Low Power Audio Subsystem (LPASS) Clock Controller"
 	select SC_GCC_7280
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 9ff4c373ad95..dce2dd639524 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -71,6 +71,7 @@ obj-$(CONFIG_SC_CAMCC_7280) += camcc-sc7280.o
 obj-$(CONFIG_SC_DISPCC_7180) += dispcc-sc7180.o
 obj-$(CONFIG_SC_DISPCC_7280) += dispcc-sc7280.o
 obj-$(CONFIG_SC_DISPCC_8280XP) += dispcc-sc8280xp.o
+obj-$(CONFIG_SC_LPASSCC_8280XP) += lpasscc-sc8280xp.o
 obj-$(CONFIG_SA_GCC_8775P) += gcc-sa8775p.o
 obj-$(CONFIG_SA_GPUCC_8775P) += gpucc-sa8775p.o
 obj-$(CONFIG_SC_GCC_7180) += gcc-sc7180.o
diff --git a/drivers/clk/qcom/lpasscc-sc8280xp.c b/drivers/clk/qcom/lpasscc-sc8280xp.c
new file mode 100644
index 000000000000..118320f8ee40
--- /dev/null
+++ b/drivers/clk/qcom/lpasscc-sc8280xp.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022, 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,lpasscc-sc8280xp.h>
+#include "common.h"
+#include "reset.h"
+
+static const struct qcom_reset_map lpass_tcsr_sc8280xp_resets[] = {
+	[LPASS_AUDIO_SWR_TX_CGCR] = { 0xc010, 1 },
+};
+
+static struct regmap_config lpass_tcsr_sc8280xp_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.name = "lpass-tcsr",
+	.max_register = 0x12000,
+};
+
+static const struct qcom_cc_desc lpass_tcsr_reset_sc8280xp_desc = {
+	.config = &lpass_tcsr_sc8280xp_regmap_config,
+	.resets = lpass_tcsr_sc8280xp_resets,
+	.num_resets = ARRAY_SIZE(lpass_tcsr_sc8280xp_resets),
+};
+
+static const struct of_device_id lpasscc_sc8280xp_match_table[] = {
+	{
+		.compatible = "qcom,sc8280xp-lpasscc",
+		.data = &lpass_tcsr_reset_sc8280xp_desc,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, lpasscc_sc8280xp_match_table);
+
+static int lpasscc_sc8280xp_probe(struct platform_device *pdev)
+{
+	const struct qcom_cc_desc *desc = of_device_get_match_data(&pdev->dev);
+
+	return qcom_cc_probe_by_index(pdev, 0, desc);
+}
+
+static struct platform_driver lpasscc_sc8280xp_driver = {
+	.probe = lpasscc_sc8280xp_probe,
+	.driver = {
+		.name = "lpasscc-sc8280xp",
+		.of_match_table = lpasscc_sc8280xp_match_table,
+	},
+};
+
+static int __init lpasscc_sc8280xp_init(void)
+{
+	return platform_driver_register(&lpasscc_sc8280xp_driver);
+}
+subsys_initcall(lpasscc_sc8280xp_init);
+
+static void __exit lpasscc_sc8280xp_exit(void)
+{
+	platform_driver_unregister(&lpasscc_sc8280xp_driver);
+}
+module_exit(lpasscc_sc8280xp_exit);
+
+MODULE_DESCRIPTION("QTI LPASSCC SC8280XP Driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1
^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 4/5] clk: qcom: Add lpass audio clock controller driver for SC8280XP
  2023-05-18 11:37 [PATCH 0/5] clk: qcom: sc8280xp: add lpasscc reset control Srinivas Kandagatla
                   ` (2 preceding siblings ...)
  2023-05-18 11:37 ` [PATCH 3/5] clk: qcom: Add lpass clock controller driver for SC8280XP Srinivas Kandagatla
@ 2023-05-18 11:37 ` Srinivas Kandagatla
  2023-05-22  8:33   ` Johan Hovold
  2023-05-18 11:38 ` [PATCH 5/5] arm64: dts: qcom: sc8280xp: add resets for soundwire controllers Srinivas Kandagatla
  4 siblings, 1 reply; 14+ messages in thread
From: Srinivas Kandagatla @ 2023-05-18 11:37 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt
  Cc: johan+linaro, agross, konrad.dybcio, mturquette, sboyd, conor+dt,
	linux-arm-msm, linux-clk, devicetree, linux-kernel,
	Srinivas Kandagatla
Add support for the lpass audio clock controller found on SC8280XP based
devices. This would allow lpass peripheral loader drivers to control the
clocks and bring the subsystems out of reset.
Currently this patch only supports resets as the Q6DSP is in control of
LPASS IP which manages most of the clocks via Q6PRM service on GPR rpmsg
channel.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/clk/qcom/lpasscc-sc8280xp.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
diff --git a/drivers/clk/qcom/lpasscc-sc8280xp.c b/drivers/clk/qcom/lpasscc-sc8280xp.c
index 118320f8ee40..e221ae2d40ae 100644
--- a/drivers/clk/qcom/lpasscc-sc8280xp.c
+++ b/drivers/clk/qcom/lpasscc-sc8280xp.c
@@ -13,6 +13,26 @@
 #include "common.h"
 #include "reset.h"
 
+static const struct qcom_reset_map lpass_audio_csr_sc8280xp_resets[] = {
+	[LPASS_AUDIO_SWR_RX_CGCR] =  { 0xa0, 1 },
+	[LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
+	[LPASS_AUDIO_SWR_WSA2_CGCR] =  { 0xd8, 1 },
+};
+
+static struct regmap_config lpass_audio_csr_sc8280xp_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_audio_csr_reset_sc8280xp_desc = {
+	.config = &lpass_audio_csr_sc8280xp_regmap_config,
+	.resets = lpass_audio_csr_sc8280xp_resets,
+	.num_resets = ARRAY_SIZE(lpass_audio_csr_sc8280xp_resets),
+};
+
 static const struct qcom_reset_map lpass_tcsr_sc8280xp_resets[] = {
 	[LPASS_AUDIO_SWR_TX_CGCR] = { 0xc010, 1 },
 };
@@ -33,6 +53,9 @@ static const struct qcom_cc_desc lpass_tcsr_reset_sc8280xp_desc = {
 
 static const struct of_device_id lpasscc_sc8280xp_match_table[] = {
 	{
+		.compatible = "qcom,sc8280xp-lpassaudiocc",
+		.data = &lpass_audio_csr_reset_sc8280xp_desc,
+	}, {
 		.compatible = "qcom,sc8280xp-lpasscc",
 		.data = &lpass_tcsr_reset_sc8280xp_desc,
 	},
-- 
2.25.1
^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 5/5] arm64: dts: qcom: sc8280xp: add resets for soundwire controllers
  2023-05-18 11:37 [PATCH 0/5] clk: qcom: sc8280xp: add lpasscc reset control Srinivas Kandagatla
                   ` (3 preceding siblings ...)
  2023-05-18 11:37 ` [PATCH 4/5] clk: qcom: Add lpass audio " Srinivas Kandagatla
@ 2023-05-18 11:38 ` Srinivas Kandagatla
  2023-05-22  8:42   ` Johan Hovold
  4 siblings, 1 reply; 14+ messages in thread
From: Srinivas Kandagatla @ 2023-05-18 11:38 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt
  Cc: johan+linaro, agross, konrad.dybcio, mturquette, sboyd, conor+dt,
	linux-arm-msm, linux-clk, devicetree, linux-kernel,
	Srinivas Kandagatla
Soundwire controllers on sc8280xp needs an explicit reset, this
patch adds support for this.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index d2a2224d138a..a2d0f8abe23d 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -6,6 +6,7 @@
 
 #include <dt-bindings/clock/qcom,dispcc-sc8280xp.h>
 #include <dt-bindings/clock/qcom,gcc-sc8280xp.h>
+#include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
 #include <dt-bindings/clock/qcom,rpmh.h>
 #include <dt-bindings/interconnect/qcom,osm-l3.h>
 #include <dt-bindings/interconnect/qcom,sc8280xp.h>
@@ -2548,6 +2549,8 @@ rxmacro: rxmacro@3200000 {
 		swr1: soundwire-controller@3210000 {
 			compatible = "qcom,soundwire-v1.6.0";
 			reg = <0 0x03210000 0 0x2000>;
+			resets = <&lpass_audiocc LPASS_AUDIO_SWR_RX_CGCR>;
+			reset-names = "swr_audio_cgcr";
 			interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&rxmacro>;
 			clock-names = "iface";
@@ -2647,6 +2650,13 @@ swr0: soundwire-controller@3250000 {
 			status = "disabled";
 		};
 
+		lpass_audiocc: clock-controller@3300000 {
+			compatible = "qcom,sc8280xp-lpassaudiocc";
+			reg = <0 0x032a9000 0 0x1000>;
+			#reset-cells = <1>;
+			#clock-cells = <1>;
+		};
+
 		swr2: soundwire-controller@3330000 {
 			compatible = "qcom,soundwire-v1.6.0";
 			reg = <0 0x03330000 0 0x2000>;
@@ -2654,6 +2664,8 @@ swr2: soundwire-controller@3330000 {
 				     <GIC_SPI 520 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "core", "wakeup";
 
+			resets = <&lpasscc LPASS_AUDIO_SWR_TX_CGCR>;
+			reset-names = "swr_audio_cgcr";
 			clocks = <&txmacro>;
 			clock-names = "iface";
 			label = "TX";
@@ -2849,6 +2861,13 @@ data-pins {
 			};
 		};
 
+		lpasscc: clock-controller@33e0000 {
+			compatible = "qcom,sc8280xp-lpasscc";
+			reg = <0 0x033e0000 0 0x21000>;
+			#reset-cells = <1>;
+			#clock-cells = <1>;
+		};
+
 		usb_0_qmpphy: phy@88eb000 {
 			compatible = "qcom,sc8280xp-qmp-usb43dp-phy";
 			reg = <0 0x088eb000 0 0x4000>;
-- 
2.25.1
^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] dt-bindings: clock: Add YAML schemas for LPASSCC and reset on SC8280XP
  2023-05-18 11:37 ` [PATCH 1/5] dt-bindings: clock: Add YAML schemas for LPASSCC and reset on SC8280XP Srinivas Kandagatla
@ 2023-05-22  8:21   ` Johan Hovold
  2023-05-25  9:21     ` Srinivas Kandagatla
  0 siblings, 1 reply; 14+ messages in thread
From: Johan Hovold @ 2023-05-22  8:21 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, johan+linaro, agross,
	konrad.dybcio, mturquette, sboyd, conor+dt, linux-arm-msm,
	linux-clk, devicetree, linux-kernel
On Thu, May 18, 2023 at 12:37:56PM +0100, Srinivas Kandagatla wrote:
> The LPASS(Low Power Audio Subsystem) clock provider provides reset
Missing space after LPASS acronym.
s/provider/controller/?
> controller support when is driven by the Q6DSP.
s/controller//?
"when is driven by": sounds like there are some words missing here.
> This patch adds support for those resets and adds IDs for clients
There is never any need to say "this patch" in a commit message. Just say
	Add support for...
> to request the reset.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  .../bindings/clock/qcom,sc8280xp-lpasscc.yaml | 57 +++++++++++++++++++
>  .../dt-bindings/clock/qcom,lpasscc-sc8280xp.h | 12 ++++
>  2 files changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
>  create mode 100644 include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
> new file mode 100644
> index 000000000000..7c30614a0af9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/qcom,sc8280xp-lpasscc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm LPASS Core & Audio Clock Controller on SC8280XP
> +
> +maintainers:
> +  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> +
> +description: |
> +  Qualcomm LPASS core and audio clock control module provides the clocks,
> +  reset and power domains on SC8280XP.
"power domains"? copy-paste error?
> +
> +  See also::
> +    include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
> +
> +properties:
> +  reg: true
> +
> +  compatible:
> +    enum:
> +      - qcom,sc8280xp-lpasscc
> +
> +  '#reset-cells':
> +    const: 1
> +
> +  '#clock-cells':
> +    const: 1
Move above #reset-cells for some sorting of related attributes. Same
below (in two places).
> +
> +  qcom,adsp-pil-mode:
> +    description:
> +      Indicates if the LPASS would be brought out of reset using
> +      peripheral loader.
> +    type: boolean
Move above the provider cells properties?
> +
> +required:
> +  - compatible
> +  - reg
> +  - qcom,adsp-pil-mode
If this boolean property is always needed, shouldn't that simply be
handled by the driver based on the compatible?
> +  - '#reset-cells'
> +  - '#clock-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
> +    lpasscc: clock-controller@3900000 {
> +      compatible = "qcom,sc8280xp-lpasscc";
binding examples use 4-space indentation.
> +      reg = <0x033e0000 0x12000>;
Does not match the node unit address.
> +      #reset-cells = <1>;
> +      #clock-cells = <1>;
> +      qcom,adsp-pil-mode;
> +    };
> +...
> diff --git a/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h b/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
> new file mode 100644
> index 000000000000..df800ea2741c
> --- /dev/null
> +++ b/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (c) 2023, Linaro Ltd.
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_QCOM_LPASSCC_SC8280XP_H
> +#define _DT_BINDINGS_CLK_QCOM_LPASSCC_SC8280XP_H
> +
> +/* LPASS TCSR */
> +#define LPASS_AUDIO_SWR_TX_CGCR				0
> +
> +#endif
^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/5] dt-bindings: clock: Add YAML schemas for LPASS AUDIOCC and reset on SC8280XP
  2023-05-18 11:37 ` [PATCH 2/5] dt-bindings: clock: Add YAML schemas for LPASS AUDIOCC " Srinivas Kandagatla
@ 2023-05-22  8:24   ` Johan Hovold
  0 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2023-05-22  8:24 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, johan+linaro, agross,
	konrad.dybcio, mturquette, sboyd, conor+dt, linux-arm-msm,
	linux-clk, devicetree, linux-kernel
On Thu, May 18, 2023 at 12:37:57PM +0100, Srinivas Kandagatla wrote:
> The LPASS(Low Power Audio Subsystem) Audio clock provider provides reset
Missing space after "LPASS".
Same comments as on previous patch seem to apply to the reset of the
commit message.
> controller support when is driven by the Q6DSP.
> This patch adds support for those resets and adds IDs for clients
> to request the reset.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  .../bindings/clock/qcom,sc8280xp-lpasscc.yaml         | 11 +++++++++++
>  include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h     |  5 +++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
> index 7c30614a0af9..394833819ba3 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
> @@ -22,6 +22,7 @@ properties:
>    compatible:
>      enum:
>        - qcom,sc8280xp-lpasscc
> +      - qcom,sc8280xp-lpassaudiocc
Add before lpasscc to maintain alphabetical sorting.
>  
>    '#reset-cells':
>      const: 1
> @@ -45,6 +46,16 @@ required:
>  additionalProperties: false
>  
>  examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
> +    lpass_audiocc: clock-controller@3300000 {
> +      compatible = "qcom,sc8280xp-lpassaudiocc";
4-space indentation.
> +      reg = <0x32a9000 0x1000>;
Does not match node unit address either.
> +      #reset-cells = <1>;
> +      #clock-cells = <1>;
> +      qcom,adsp-pil-mode;
Keep cells provider cells properties last?
> +    };
> +
>    - |
>      #include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
>      lpasscc: clock-controller@3900000 {
> diff --git a/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h b/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
> index df800ea2741c..d190d57fc81a 100644
> --- a/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
> +++ b/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
> @@ -6,6 +6,11 @@
>  #ifndef _DT_BINDINGS_CLK_QCOM_LPASSCC_SC8280XP_H
>  #define _DT_BINDINGS_CLK_QCOM_LPASSCC_SC8280XP_H
>  
> +/* LPASS AUDIO CC CSR */
> +#define LPASS_AUDIO_SWR_RX_CGCR				0
> +#define LPASS_AUDIO_SWR_WSA_CGCR			1
> +#define LPASS_AUDIO_SWR_WSA2_CGCR			2
> +
>  /* LPASS TCSR */
>  #define LPASS_AUDIO_SWR_TX_CGCR				0
Johan
^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 3/5] clk: qcom: Add lpass clock controller driver for SC8280XP
  2023-05-18 11:37 ` [PATCH 3/5] clk: qcom: Add lpass clock controller driver for SC8280XP Srinivas Kandagatla
@ 2023-05-22  8:31   ` Johan Hovold
  2023-05-22  8:45   ` Johan Hovold
  1 sibling, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2023-05-22  8:31 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, johan+linaro, agross,
	konrad.dybcio, mturquette, sboyd, conor+dt, linux-arm-msm,
	linux-clk, devicetree, linux-kernel
On Thu, May 18, 2023 at 12:37:58PM +0100, Srinivas Kandagatla wrote:
> Add support for the lpass clock controller found on SC8280XP based devices.
> This would allow lpass peripheral loader drivers to control the clocks and
> bring the subsystems out of reset.
> 
> Currently this patch only supports resets as the Q6DSP is in control of
> LPASS IP which manages most of the clocks via Q6PRM service on GPR rpmsg
> channel.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  drivers/clk/qcom/Kconfig            |  8 ++++
>  drivers/clk/qcom/Makefile           |  1 +
>  drivers/clk/qcom/lpasscc-sc8280xp.c | 71 +++++++++++++++++++++++++++++
>  3 files changed, 80 insertions(+)
>  create mode 100644 drivers/clk/qcom/lpasscc-sc8280xp.c
> 
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 12be3e2371b3..8188f4dedf40 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -514,6 +514,14 @@ config SC_GPUCC_8280XP
>  	  Say Y if you want to support graphics controller devices and
>  	  functionality such as 3D graphics.
>  
> +config SC_LPASSCC_8280XP
Should go after SC_LPASSCC_7280.
> +	tristate "SC8280 Low Power Audio Subsystem (LPASS) Clock Controller"
> +	select SC_GCC_8280XP
> +	help
> +	  Support for the LPASS clock controller on SC8280XP devices.
> +	  Say Y if you want to use the LPASS branch clocks of the LPASS clock
> +	  controller to reset the LPASS subsystem.
> +
>  config SC_LPASSCC_7280
>  	tristate "SC7280 Low Power Audio Subsystem (LPASS) Clock Controller"
>  	select SC_GCC_7280
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 9ff4c373ad95..dce2dd639524 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -71,6 +71,7 @@ obj-$(CONFIG_SC_CAMCC_7280) += camcc-sc7280.o
>  obj-$(CONFIG_SC_DISPCC_7180) += dispcc-sc7180.o
>  obj-$(CONFIG_SC_DISPCC_7280) += dispcc-sc7280.o
>  obj-$(CONFIG_SC_DISPCC_8280XP) += dispcc-sc8280xp.o
> +obj-$(CONFIG_SC_LPASSCC_8280XP) += lpasscc-sc8280xp.o
This looks misplaced too.
>  obj-$(CONFIG_SA_GCC_8775P) += gcc-sa8775p.o
>  obj-$(CONFIG_SA_GPUCC_8775P) += gpucc-sa8775p.o
>  obj-$(CONFIG_SC_GCC_7180) += gcc-sc7180.o
> diff --git a/drivers/clk/qcom/lpasscc-sc8280xp.c b/drivers/clk/qcom/lpasscc-sc8280xp.c
> new file mode 100644
> index 000000000000..118320f8ee40
> --- /dev/null
> +++ b/drivers/clk/qcom/lpasscc-sc8280xp.c
> @@ -0,0 +1,71 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2022, 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,lpasscc-sc8280xp.h>
> +#include "common.h"
> +#include "reset.h"
Nit: add newline separators before dt-bindings and local includes,
respectively?
> +static int __init lpasscc_sc8280xp_init(void)
> +{
> +	return platform_driver_register(&lpasscc_sc8280xp_driver);
> +}
> +subsys_initcall(lpasscc_sc8280xp_init);
Do you really need subsys init for this? I've been using this driver as
a module on the X13s and it seems to work fine.
> +static void __exit lpasscc_sc8280xp_exit(void)
> +{
> +	platform_driver_unregister(&lpasscc_sc8280xp_driver);
> +}
> +module_exit(lpasscc_sc8280xp_exit);
> +
> +MODULE_DESCRIPTION("QTI LPASSCC SC8280XP Driver");
> +MODULE_LICENSE("GPL");
Johan
^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] clk: qcom: Add lpass audio clock controller driver for SC8280XP
  2023-05-18 11:37 ` [PATCH 4/5] clk: qcom: Add lpass audio " Srinivas Kandagatla
@ 2023-05-22  8:33   ` Johan Hovold
  2023-05-25 10:31     ` Srinivas Kandagatla
  0 siblings, 1 reply; 14+ messages in thread
From: Johan Hovold @ 2023-05-22  8:33 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, johan+linaro, agross,
	konrad.dybcio, mturquette, sboyd, conor+dt, linux-arm-msm,
	linux-clk, devicetree, linux-kernel
On Thu, May 18, 2023 at 12:37:59PM +0100, Srinivas Kandagatla wrote:
> Add support for the lpass audio clock controller found on SC8280XP based
> devices. This would allow lpass peripheral loader drivers to control the
> clocks and bring the subsystems out of reset.
> 
> Currently this patch only supports resets as the Q6DSP is in control of
> LPASS IP which manages most of the clocks via Q6PRM service on GPR rpmsg
> channel.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  drivers/clk/qcom/lpasscc-sc8280xp.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/clk/qcom/lpasscc-sc8280xp.c b/drivers/clk/qcom/lpasscc-sc8280xp.c
> index 118320f8ee40..e221ae2d40ae 100644
> --- a/drivers/clk/qcom/lpasscc-sc8280xp.c
> +++ b/drivers/clk/qcom/lpasscc-sc8280xp.c
> @@ -13,6 +13,26 @@
>  #include "common.h"
>  #include "reset.h"
>  
> +static const struct qcom_reset_map lpass_audio_csr_sc8280xp_resets[] = {
> +	[LPASS_AUDIO_SWR_RX_CGCR] =  { 0xa0, 1 },
> +	[LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
> +	[LPASS_AUDIO_SWR_WSA2_CGCR] =  { 0xd8, 1 },
> +};
> +
> +static struct regmap_config lpass_audio_csr_sc8280xp_regmap_config = {
> +	.reg_bits = 32,
> +	.reg_stride = 4,
> +	.val_bits = 32,
> +	.name = "lpass-audio-csr",
Should you update this name to match the new compatible
("lpassaudiocc")?
> +	.max_register = 0x1000,
> +};
> +
> +static const struct qcom_cc_desc lpass_audio_csr_reset_sc8280xp_desc = {
Same here (and for the reset struct as well as previous patch).
> +	.config = &lpass_audio_csr_sc8280xp_regmap_config,
> +	.resets = lpass_audio_csr_sc8280xp_resets,
> +	.num_resets = ARRAY_SIZE(lpass_audio_csr_sc8280xp_resets),
> +};
> +
>  static const struct qcom_reset_map lpass_tcsr_sc8280xp_resets[] = {
>  	[LPASS_AUDIO_SWR_TX_CGCR] = { 0xc010, 1 },
>  };
> @@ -33,6 +53,9 @@ static const struct qcom_cc_desc lpass_tcsr_reset_sc8280xp_desc = {
>  
>  static const struct of_device_id lpasscc_sc8280xp_match_table[] = {
>  	{
> +		.compatible = "qcom,sc8280xp-lpassaudiocc",
> +		.data = &lpass_audio_csr_reset_sc8280xp_desc,
> +	}, {
>  		.compatible = "qcom,sc8280xp-lpasscc",
>  		.data = &lpass_tcsr_reset_sc8280xp_desc,
>  	},
Johan
^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 5/5] arm64: dts: qcom: sc8280xp: add resets for soundwire controllers
  2023-05-18 11:38 ` [PATCH 5/5] arm64: dts: qcom: sc8280xp: add resets for soundwire controllers Srinivas Kandagatla
@ 2023-05-22  8:42   ` Johan Hovold
  0 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2023-05-22  8:42 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, johan+linaro, agross,
	konrad.dybcio, mturquette, sboyd, conor+dt, linux-arm-msm,
	linux-clk, devicetree, linux-kernel
On Thu, May 18, 2023 at 12:38:00PM +0100, Srinivas Kandagatla wrote:
> Soundwire controllers on sc8280xp needs an explicit reset, this
> patch adds support for this.
s/this patch adds/add/
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index d2a2224d138a..a2d0f8abe23d 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> @@ -6,6 +6,7 @@
>  
>  #include <dt-bindings/clock/qcom,dispcc-sc8280xp.h>
>  #include <dt-bindings/clock/qcom,gcc-sc8280xp.h>
> +#include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
>  #include <dt-bindings/clock/qcom,rpmh.h>
>  #include <dt-bindings/interconnect/qcom,osm-l3.h>
>  #include <dt-bindings/interconnect/qcom,sc8280xp.h>
> @@ -2548,6 +2549,8 @@ rxmacro: rxmacro@3200000 {
>  		swr1: soundwire-controller@3210000 {
>  			compatible = "qcom,soundwire-v1.6.0";
>  			reg = <0 0x03210000 0 0x2000>;
> +			resets = <&lpass_audiocc LPASS_AUDIO_SWR_RX_CGCR>;
> +			reset-names = "swr_audio_cgcr";
Move after clocks.
>  			interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
>  			clocks = <&rxmacro>;
>  			clock-names = "iface";
> @@ -2647,6 +2650,13 @@ swr0: soundwire-controller@3250000 {
>  			status = "disabled";
>  		};
>  
> +		lpass_audiocc: clock-controller@3300000 {
> +			compatible = "qcom,sc8280xp-lpassaudiocc";
> +			reg = <0 0x032a9000 0 0x1000>;
Either this property or the unit address is wrong as they do not match.
The bindings currently mandates that vendor property you added
("qcom,adsp-pil-mode), but you left it out here.
> +			#reset-cells = <1>;
> +			#clock-cells = <1>;
clock before reset for some sort order.
> +		};
> +
Your preliminary version of this patch also added a reset to swr0, which
has been left out here. Was that not needed?
>  		swr2: soundwire-controller@3330000 {
>  			compatible = "qcom,soundwire-v1.6.0";
>  			reg = <0 0x03330000 0 0x2000>;
> @@ -2654,6 +2664,8 @@ swr2: soundwire-controller@3330000 {
>  				     <GIC_SPI 520 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "core", "wakeup";
>  
> +			resets = <&lpasscc LPASS_AUDIO_SWR_TX_CGCR>;
> +			reset-names = "swr_audio_cgcr";
Add after clocks.
>  			clocks = <&txmacro>;
>  			clock-names = "iface";
>  			label = "TX";
> @@ -2849,6 +2861,13 @@ data-pins {
>  			};
>  		};
>  
> +		lpasscc: clock-controller@33e0000 {
> +			compatible = "qcom,sc8280xp-lpasscc";
> +			reg = <0 0x033e0000 0 0x21000>;
Your driver (and the binding example) seems to suggest that the size
here should be 0x12000.
The vendor property appears to be missing here too (or the binding is
incorrect).
> +			#reset-cells = <1>;
> +			#clock-cells = <1>;
clock before reset
> +		};
> +
>  		usb_0_qmpphy: phy@88eb000 {
>  			compatible = "qcom,sc8280xp-qmp-usb43dp-phy";
>  			reg = <0 0x088eb000 0 0x4000>;
Johan
^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 3/5] clk: qcom: Add lpass clock controller driver for SC8280XP
  2023-05-18 11:37 ` [PATCH 3/5] clk: qcom: Add lpass clock controller driver for SC8280XP Srinivas Kandagatla
  2023-05-22  8:31   ` Johan Hovold
@ 2023-05-22  8:45   ` Johan Hovold
  1 sibling, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2023-05-22  8:45 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, johan+linaro, agross,
	konrad.dybcio, mturquette, sboyd, conor+dt, linux-arm-msm,
	linux-clk, devicetree, linux-kernel
On Thu, May 18, 2023 at 12:37:58PM +0100, Srinivas Kandagatla wrote:
> +config SC_LPASSCC_8280XP
> +	tristate "SC8280 Low Power Audio Subsystem (LPASS) Clock Controller"
> +	select SC_GCC_8280XP
> +	help
> +	  Support for the LPASS clock controller on SC8280XP devices.
> +	  Say Y if you want to use the LPASS branch clocks of the LPASS clock
> +	  controller to reset the LPASS subsystem.
And please include a defconfig update for this one as a separate patch
in the next revision as it is needed for audio on the X13s.
Johan
^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] dt-bindings: clock: Add YAML schemas for LPASSCC and reset on SC8280XP
  2023-05-22  8:21   ` Johan Hovold
@ 2023-05-25  9:21     ` Srinivas Kandagatla
  0 siblings, 0 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2023-05-25  9:21 UTC (permalink / raw)
  To: Johan Hovold
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, johan+linaro, agross,
	konrad.dybcio, mturquette, sboyd, conor+dt, linux-arm-msm,
	linux-clk, devicetree, linux-kernel
thanks Johan for review,
On 22/05/2023 09:21, Johan Hovold wrote:
> On Thu, May 18, 2023 at 12:37:56PM +0100, Srinivas Kandagatla wrote:
>> The LPASS(Low Power Audio Subsystem) clock provider provides reset
> 
> Missing space after LPASS acronym.
> 
> s/provider/controller/?
> 
>> controller support when is driven by the Q6DSP.
> 
> s/controller//?
> 
> "when is driven by": sounds like there are some words missing here.
> 
>> This patch adds support for those resets and adds IDs for clients
> 
> There is never any need to say "this patch" in a commit message. Just say
> 
> 	Add support for...
> 
>> to request the reset.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   .../bindings/clock/qcom,sc8280xp-lpasscc.yaml | 57 +++++++++++++++++++
>>   .../dt-bindings/clock/qcom,lpasscc-sc8280xp.h | 12 ++++
>>   2 files changed, 69 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
>>   create mode 100644 include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
>> new file mode 100644
>> index 000000000000..7c30614a0af9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
>> @@ -0,0 +1,57 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/qcom,sc8280xp-lpasscc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm LPASS Core & Audio Clock Controller on SC8280XP
>> +
>> +maintainers:
>> +  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> +
>> +description: |
>> +  Qualcomm LPASS core and audio clock control module provides the clocks,
>> +  reset and power domains on SC8280XP.
> 
> "power domains"? copy-paste error?
> 
>> +
>> +  See also::
>> +    include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
>> +
>> +properties:
>> +  reg: true
>> +
>> +  compatible:
>> +    enum:
>> +      - qcom,sc8280xp-lpasscc
>> +
>> +  '#reset-cells':
>> +    const: 1
>> +
>> +  '#clock-cells':
>> +    const: 1
> 
> Move above #reset-cells for some sorting of related attributes. Same
> below (in two places).
> 
>> +
>> +  qcom,adsp-pil-mode:
>> +    description:
>> +      Indicates if the LPASS would be brought out of reset using
>> +      peripheral loader.
>> +    type: boolean
> 
> Move above the provider cells properties?
> 
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - qcom,adsp-pil-mode
> 
> If this boolean property is always needed, shouldn't that simply be
> handled by the driver based on the compatible?
Traditionally in Qcom SoCs LPASS is under the control of ADSP, there 
have been some other variants specially chrome platforms that have moved 
this control to APPs processor.
Having this property at Device tree level provides more flexibility, 
given that both the cases use same compatible strings.
Am okay with reset of the comments, Will fix them in v2.
thanks,
Srini
> 
>> +  - '#reset-cells'
>> +  - '#clock-cells'
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
>> +    lpasscc: clock-controller@3900000 {
>> +      compatible = "qcom,sc8280xp-lpasscc";
> 
> binding examples use 4-space indentation.
> 
>> +      reg = <0x033e0000 0x12000>;
> 
> Does not match the node unit address.
> 
>> +      #reset-cells = <1>;
>> +      #clock-cells = <1>;
>> +      qcom,adsp-pil-mode;
>> +    };
>> +...
>> diff --git a/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h b/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
>> new file mode 100644
>> index 000000000000..df800ea2741c
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
>> @@ -0,0 +1,12 @@
>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>> +/*
>> + * Copyright (c) 2023, Linaro Ltd.
>> + */
>> +
>> +#ifndef _DT_BINDINGS_CLK_QCOM_LPASSCC_SC8280XP_H
>> +#define _DT_BINDINGS_CLK_QCOM_LPASSCC_SC8280XP_H
>> +
>> +/* LPASS TCSR */
>> +#define LPASS_AUDIO_SWR_TX_CGCR				0
>> +
>> +#endif
^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] clk: qcom: Add lpass audio clock controller driver for SC8280XP
  2023-05-22  8:33   ` Johan Hovold
@ 2023-05-25 10:31     ` Srinivas Kandagatla
  0 siblings, 0 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2023-05-25 10:31 UTC (permalink / raw)
  To: Johan Hovold
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, johan+linaro, agross,
	konrad.dybcio, mturquette, sboyd, conor+dt, linux-arm-msm,
	linux-clk, devicetree, linux-kernel
On 22/05/2023 09:33, Johan Hovold wrote:
> On Thu, May 18, 2023 at 12:37:59PM +0100, Srinivas Kandagatla wrote:
>> Add support for the lpass audio clock controller found on SC8280XP based
>> devices. This would allow lpass peripheral loader drivers to control the
>> clocks and bring the subsystems out of reset.
>>
>> Currently this patch only supports resets as the Q6DSP is in control of
>> LPASS IP which manages most of the clocks via Q6PRM service on GPR rpmsg
>> channel.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   drivers/clk/qcom/lpasscc-sc8280xp.c | 23 +++++++++++++++++++++++
>>   1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/lpasscc-sc8280xp.c b/drivers/clk/qcom/lpasscc-sc8280xp.c
>> index 118320f8ee40..e221ae2d40ae 100644
>> --- a/drivers/clk/qcom/lpasscc-sc8280xp.c
>> +++ b/drivers/clk/qcom/lpasscc-sc8280xp.c
>> @@ -13,6 +13,26 @@
>>   #include "common.h"
>>   #include "reset.h"
>>   
>> +static const struct qcom_reset_map lpass_audio_csr_sc8280xp_resets[] = {
>> +	[LPASS_AUDIO_SWR_RX_CGCR] =  { 0xa0, 1 },
>> +	[LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
>> +	[LPASS_AUDIO_SWR_WSA2_CGCR] =  { 0xd8, 1 },
>> +};
>> +
>> +static struct regmap_config lpass_audio_csr_sc8280xp_regmap_config = {
>> +	.reg_bits = 32,
>> +	.reg_stride = 4,
>> +	.val_bits = 32,
>> +	.name = "lpass-audio-csr",
> 
> Should you update this name to match the new compatible
> ("lpassaudiocc")?
This name reflects the name from data sheet, keeping it that way would 
be useful.
--srini
> 
>> +	.max_register = 0x1000,
>> +};
>> +
>> +static const struct qcom_cc_desc lpass_audio_csr_reset_sc8280xp_desc = {
> 
> Same here (and for the reset struct as well as previous patch).
> 
>> +	.config = &lpass_audio_csr_sc8280xp_regmap_config,
>> +	.resets = lpass_audio_csr_sc8280xp_resets,
>> +	.num_resets = ARRAY_SIZE(lpass_audio_csr_sc8280xp_resets),
>> +};
>> +
>>   static const struct qcom_reset_map lpass_tcsr_sc8280xp_resets[] = {
>>   	[LPASS_AUDIO_SWR_TX_CGCR] = { 0xc010, 1 },
>>   };
>> @@ -33,6 +53,9 @@ static const struct qcom_cc_desc lpass_tcsr_reset_sc8280xp_desc = {
>>   
>>   static const struct of_device_id lpasscc_sc8280xp_match_table[] = {
>>   	{
>> +		.compatible = "qcom,sc8280xp-lpassaudiocc",
>> +		.data = &lpass_audio_csr_reset_sc8280xp_desc,
>> +	}, {
>>   		.compatible = "qcom,sc8280xp-lpasscc",
>>   		.data = &lpass_tcsr_reset_sc8280xp_desc,
>>   	},
> 
> Johan
^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-05-25 10:31 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-18 11:37 [PATCH 0/5] clk: qcom: sc8280xp: add lpasscc reset control Srinivas Kandagatla
2023-05-18 11:37 ` [PATCH 1/5] dt-bindings: clock: Add YAML schemas for LPASSCC and reset on SC8280XP Srinivas Kandagatla
2023-05-22  8:21   ` Johan Hovold
2023-05-25  9:21     ` Srinivas Kandagatla
2023-05-18 11:37 ` [PATCH 2/5] dt-bindings: clock: Add YAML schemas for LPASS AUDIOCC " Srinivas Kandagatla
2023-05-22  8:24   ` Johan Hovold
2023-05-18 11:37 ` [PATCH 3/5] clk: qcom: Add lpass clock controller driver for SC8280XP Srinivas Kandagatla
2023-05-22  8:31   ` Johan Hovold
2023-05-22  8:45   ` Johan Hovold
2023-05-18 11:37 ` [PATCH 4/5] clk: qcom: Add lpass audio " Srinivas Kandagatla
2023-05-22  8:33   ` Johan Hovold
2023-05-25 10:31     ` Srinivas Kandagatla
2023-05-18 11:38 ` [PATCH 5/5] arm64: dts: qcom: sc8280xp: add resets for soundwire controllers Srinivas Kandagatla
2023-05-22  8:42   ` Johan Hovold
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).