Re: [PATCH 1/5] dt-bindings: clock: Add YAML schemas for LPASSCC and reset on SC8280XP

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

 



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@xxxxxxxxxx>
---
  .../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@xxxxxxxxxx>
+
+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



[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