diff mbox series

[v2,2/3] dt-bindings: arm: Adds CoreSight CSR hardware definitions

Message ID 20230813151253.38128-3-quic_jinlmao@quicinc.com (mailing list archive)
State Not Applicable
Headers show
Series Add support for a streaming interface for TMC ETR | expand

Commit Message

Mao Jinlong Aug. 13, 2023, 3:12 p.m. UTC
Adds new coresight-csr.yaml file describing the bindings required
to define csr in the device trees.

Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
---
 .../bindings/arm/qcom,coresight-csr.yaml      | 130 ++++++++++++++++++
 MAINTAINERS                                   |   2 +-
 include/dt-bindings/arm/coresight-csr-dt.h    |  12 ++
 3 files changed, 143 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-csr.yaml
 create mode 100644 include/dt-bindings/arm/coresight-csr-dt.h

Comments

Rob Herring Aug. 21, 2023, 5:54 p.m. UTC | #1
On Sun, Aug 13, 2023 at 11:12:52PM +0800, Mao Jinlong wrote:
> Adds new coresight-csr.yaml file describing the bindings required
> to define csr in the device trees.

Why?

What is CSR? Control & Status Registers in my book.

> 
> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
> ---
>  .../bindings/arm/qcom,coresight-csr.yaml      | 130 ++++++++++++++++++
>  MAINTAINERS                                   |   2 +-
>  include/dt-bindings/arm/coresight-csr-dt.h    |  12 ++
>  3 files changed, 143 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-csr.yaml
>  create mode 100644 include/dt-bindings/arm/coresight-csr-dt.h
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-csr.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-csr.yaml
> new file mode 100644
> index 000000000000..de4baa335fdb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-csr.yaml
> @@ -0,0 +1,130 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/qcom,coresight-csr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: CoreSight Slave Register - CSR
> +
> +description: |

Don't need '|'

> +  CoreSight Slave Register block hosts miscellaneous configuration registers.
> +  Those configuration registers can be used to control, various coresight
> +  configurations.

I still have no idea what this block does other than something related 
to Coresight.

> +
> +maintainers:
> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
> +  - Hao Zhang <quic_hazha@quicinc.com>
> +
> +properties:
> +  $nodename:
> +    pattern: "^csr(@[0-9a-f]+)$"
> +  compatible:
> +    items:
> +      - const: qcom,coresight-csr
> +
> +  reg:
> +    minItems: 1
> +    maxItems: 2
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: apb_pclk
> +
> +  # size cells and address cells required if assoc_device node present.
> +  "#size-cells":
> +    const: 0
> +
> +  "#address-cells":
> +    const: 1
> +
> +patternProperties:
> +  '^assoc_device@([0-9]+)$':

Unit-addresses are typically hex.

> +    type: object
> +    description:
> +      A assocated device child node which describes the required configs
> +      between this CSR and another hardware device. This device may be ETR or
> +      TPDM device.
> +
> +    properties:
> +      reg:
> +        maxItems: 1

I don't understand what 'reg' signifies here.

> +
> +      arm,cs-dev-assoc:
> +        $ref: /schemas/types.yaml#/definitions/phandle
> +        description:
> +          defines a phandle reference to an associated CoreSight trace device.
> +          When the associated trace device is enabled, then the respective CSR
> +          will be enabled. If the associated device has not been registered
> +          then the node name will be stored as the assocated name for later
> +          resolution.

registered? Sounds like a OS detail that doesn't belong in DT.

> +
> +      qcom,cs-dev-type:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Device type of the Assocated device. Types are in coresight-csr-dt.h.

Constraints?

> +
> +      qcom,csr-bytecntr-offset:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          The ETR irqctrl register offset. If the assocated device is ETR
> +          device and there are more than one ETR devices, this property need
> +          to be added.

You should know the register offset based on the ETR compatible.

> +
> +      interrupts:
> +        minItems: 1
> +
> +      interrupt-names:
> +        minItems: 1
> +
> +    required:
> +      - reg
> +      - qcom,cs-dev-type
> +      - qcom,cs-dev-assoc
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  # minimum CSR definition.
> +  - |
> +    csr@10001000 {
> +      compatible = "qcom,coresight-csr";
> +      reg = <0 0x10001000 0 0x1000>;
> +
> +      clocks = <&aoss_qmp>;
> +      clock-names = "apb_pclk";
> +    };
> +  # Assocated with ETR device
> +  - |
> +    #include <dt-bindings/arm/coresight-csr-dt.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    csr@10001000 {
> +      compatible = "qcom,coresight-csr";
> +      reg = <0 0x10001000 0 0x1000>;
> +
> +      clocks = <&aoss_qmp>;
> +      clock-names = "apb_pclk";
> +
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      assoc_device@0 {
> +        reg = <0>;
> +        qcom,cs-dev-type = <CSR_ASSOC_DEV_ETR>;
> +        qcom,cs-dev-assoc = <&tmc_etr>;
> +        qcom,csr-bytecntr-offset = <0x6c>;
> +        interrupts = <GIC_SPI 270 IRQ_TYPE_EDGE_RISING>;
> +        interrupt-names = "byte-cntr-irq";
> +      };
> +    };
> +...
> +
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d516295978a4..3ed81a8fd1d0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2042,7 +2042,7 @@ F:	Documentation/devicetree/bindings/arm/arm,trace-buffer-extension.yaml
>  F:	Documentation/devicetree/bindings/arm/qcom,coresight-*.yaml
>  F:	Documentation/trace/coresight/*
>  F:	drivers/hwtracing/coresight/*
> -F:	include/dt-bindings/arm/coresight-cti-dt.h
> +F:	include/dt-bindings/arm/coresight-*.h
>  F:	include/linux/coresight*
>  F:	samples/coresight/*
>  F:	tools/perf/arch/arm/util/auxtrace.c
> diff --git a/include/dt-bindings/arm/coresight-csr-dt.h b/include/dt-bindings/arm/coresight-csr-dt.h
> new file mode 100644
> index 000000000000..804b9bbeb2bd
> --- /dev/null
> +++ b/include/dt-bindings/arm/coresight-csr-dt.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * This header provides constants for the defined device
> + * types on CoreSight CSR.
> + */
> +
> +#ifndef _DT_BINDINGS_ARM_CORESIGHT_CSR_DT_H
> +#define _DT_BINDINGS_ARM_CORESIGHT_CSR_DT_H
> +
> +#define CSR_ASSOC_DEV_ETR	1
> +
> +#endif /*_DT_BINDINGS_ARM_CORESIGHT_CSR_DT_H */
> -- 
> 2.17.1
>
Krzysztof Kozlowski Aug. 22, 2023, 6:04 a.m. UTC | #2
On 13/08/2023 17:12, Mao Jinlong wrote:
> Adds new coresight-csr.yaml file describing the bindings required
> to define csr in the device trees.
> 
> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
> ---
>  .../bindings/arm/qcom,coresight-csr.yaml      | 130 ++++++++++++++++++
>  MAINTAINERS                                   |   2 +-
>  include/dt-bindings/arm/coresight-csr-dt.h    |  12 ++
>  3 files changed, 143 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-csr.yaml
>  create mode 100644 include/dt-bindings/arm/coresight-csr-dt.h
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-csr.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-csr.yaml
> new file mode 100644
> index 000000000000..de4baa335fdb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-csr.yaml
> @@ -0,0 +1,130 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/qcom,coresight-csr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: CoreSight Slave Register - CSR
> +
> +description: |
> +  CoreSight Slave Register block hosts miscellaneous configuration registers.
> +  Those configuration registers can be used to control, various coresight
> +  configurations.
> +
> +maintainers:
> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
> +  - Hao Zhang <quic_hazha@quicinc.com>
> +
> +properties:
> +  $nodename:
> +    pattern: "^csr(@[0-9a-f]+)$"

Blank line. Or even drop the nodename, we do not enforce names in the
individual bindings and I do not get why "csr" should be a recommended
name. It's not really generic, but specific.

> +  compatible:
> +    items:
> +      - const: qcom,coresight-csr
> +
> +  reg:
> +    minItems: 1
> +    maxItems: 2

Why is this flexible? One device has only one register layout... or you
want to say that compatible is not specific but generic?

Anyway, items needs to be described.

> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: apb_pclk
> +
> +  # size cells and address cells required if assoc_device node present.
> +  "#size-cells":
> +    const: 0
> +
> +  "#address-cells":
> +    const: 1
> +
> +patternProperties:
> +  '^assoc_device@([0-9]+)$':

No underscores.

Aren't you now creating duplicated nodes for devices? ETRs for example
have their device nodes, right? So here you would be creating second
one? If that's the case, then it looks wrong.

> +    type: object
> +    description:
> +      A assocated device child node which describes the required configs
> +      between this CSR and another hardware device. This device may be ETR or
> +      TPDM device.
> +
> +    properties:
> +      reg:
> +        maxItems: 1
> +
> +      arm,cs-dev-assoc:
> +        $ref: /schemas/types.yaml#/definitions/phandle
> +        description:
> +          defines a phandle reference to an associated CoreSight trace device.
> +          When the associated trace device is enabled, then the respective CSR
> +          will be enabled. If the associated device has not been registered
> +          then the node name will be stored as the assocated name for later
> +          resolution.
> +
> +      qcom,cs-dev-type:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Device type of the Assocated device. Types are in coresight-csr-dt.h.
> +
> +      qcom,csr-bytecntr-offset:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          The ETR irqctrl register offset. If the assocated device is ETR
> +          device and there are more than one ETR devices, this property need
> +          to be added.
> +
> +      interrupts:
> +        minItems: 1
> +
> +      interrupt-names:
> +        minItems: 1
> +
> +    required:
> +      - reg
> +      - qcom,cs-dev-type
> +      - qcom,cs-dev-assoc
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  # minimum CSR definition.
> +  - |
> +    csr@10001000 {
> +      compatible = "qcom,coresight-csr";
> +      reg = <0 0x10001000 0 0x1000>;
> +
> +      clocks = <&aoss_qmp>;
> +      clock-names = "apb_pclk";
> +    };
> +  # Assocated with ETR device
> +  - |
> +    #include <dt-bindings/arm/coresight-csr-dt.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    csr@10001000 {
> +      compatible = "qcom,coresight-csr";
> +      reg = <0 0x10001000 0 0x1000>;
> +
> +      clocks = <&aoss_qmp>;
> +      clock-names = "apb_pclk";
> +
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      assoc_device@0 {
> +        reg = <0>;
> +        qcom,cs-dev-type = <CSR_ASSOC_DEV_ETR>;
> +        qcom,cs-dev-assoc = <&tmc_etr>;
> +        qcom,csr-bytecntr-offset = <0x6c>;
> +        interrupts = <GIC_SPI 270 IRQ_TYPE_EDGE_RISING>;
> +        interrupt-names = "byte-cntr-irq";
> +      };
> +    };
> +...
> +
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d516295978a4..3ed81a8fd1d0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2042,7 +2042,7 @@ F:	Documentation/devicetree/bindings/arm/arm,trace-buffer-extension.yaml
>  F:	Documentation/devicetree/bindings/arm/qcom,coresight-*.yaml
>  F:	Documentation/trace/coresight/*
>  F:	drivers/hwtracing/coresight/*
> -F:	include/dt-bindings/arm/coresight-cti-dt.h
> +F:	include/dt-bindings/arm/coresight-*.h
>  F:	include/linux/coresight*
>  F:	samples/coresight/*
>  F:	tools/perf/arch/arm/util/auxtrace.c
> diff --git a/include/dt-bindings/arm/coresight-csr-dt.h b/include/dt-bindings/arm/coresight-csr-dt.h
> new file mode 100644
> index 000000000000..804b9bbeb2bd
> --- /dev/null
> +++ b/include/dt-bindings/arm/coresight-csr-dt.h

Use the same naming pattern as for bindings, so qcom,coresight-csr if it
is qcom, or arm,coresight-csr if it is ARM. DT is for sure redundant.

> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * This header provides constants for the defined device
> + * types on CoreSight CSR.
> + */
> +
> +#ifndef _DT_BINDINGS_ARM_CORESIGHT_CSR_DT_H
> +#define _DT_BINDINGS_ARM_CORESIGHT_CSR_DT_H
> +
> +#define CSR_ASSOC_DEV_ETR	1
> +
> +#endif /*_DT_BINDINGS_ARM_CORESIGHT_CSR_DT_H */

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-csr.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-csr.yaml
new file mode 100644
index 000000000000..de4baa335fdb
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/qcom,coresight-csr.yaml
@@ -0,0 +1,130 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/qcom,coresight-csr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: CoreSight Slave Register - CSR
+
+description: |
+  CoreSight Slave Register block hosts miscellaneous configuration registers.
+  Those configuration registers can be used to control, various coresight
+  configurations.
+
+maintainers:
+  - Mao Jinlong <quic_jinlmao@quicinc.com>
+  - Hao Zhang <quic_hazha@quicinc.com>
+
+properties:
+  $nodename:
+    pattern: "^csr(@[0-9a-f]+)$"
+  compatible:
+    items:
+      - const: qcom,coresight-csr
+
+  reg:
+    minItems: 1
+    maxItems: 2
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: apb_pclk
+
+  # size cells and address cells required if assoc_device node present.
+  "#size-cells":
+    const: 0
+
+  "#address-cells":
+    const: 1
+
+patternProperties:
+  '^assoc_device@([0-9]+)$':
+    type: object
+    description:
+      A assocated device child node which describes the required configs
+      between this CSR and another hardware device. This device may be ETR or
+      TPDM device.
+
+    properties:
+      reg:
+        maxItems: 1
+
+      arm,cs-dev-assoc:
+        $ref: /schemas/types.yaml#/definitions/phandle
+        description:
+          defines a phandle reference to an associated CoreSight trace device.
+          When the associated trace device is enabled, then the respective CSR
+          will be enabled. If the associated device has not been registered
+          then the node name will be stored as the assocated name for later
+          resolution.
+
+      qcom,cs-dev-type:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          Device type of the Assocated device. Types are in coresight-csr-dt.h.
+
+      qcom,csr-bytecntr-offset:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          The ETR irqctrl register offset. If the assocated device is ETR
+          device and there are more than one ETR devices, this property need
+          to be added.
+
+      interrupts:
+        minItems: 1
+
+      interrupt-names:
+        minItems: 1
+
+    required:
+      - reg
+      - qcom,cs-dev-type
+      - qcom,cs-dev-assoc
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  # minimum CSR definition.
+  - |
+    csr@10001000 {
+      compatible = "qcom,coresight-csr";
+      reg = <0 0x10001000 0 0x1000>;
+
+      clocks = <&aoss_qmp>;
+      clock-names = "apb_pclk";
+    };
+  # Assocated with ETR device
+  - |
+    #include <dt-bindings/arm/coresight-csr-dt.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    csr@10001000 {
+      compatible = "qcom,coresight-csr";
+      reg = <0 0x10001000 0 0x1000>;
+
+      clocks = <&aoss_qmp>;
+      clock-names = "apb_pclk";
+
+      #address-cells = <1>;
+      #size-cells = <0>;
+      assoc_device@0 {
+        reg = <0>;
+        qcom,cs-dev-type = <CSR_ASSOC_DEV_ETR>;
+        qcom,cs-dev-assoc = <&tmc_etr>;
+        qcom,csr-bytecntr-offset = <0x6c>;
+        interrupts = <GIC_SPI 270 IRQ_TYPE_EDGE_RISING>;
+        interrupt-names = "byte-cntr-irq";
+      };
+    };
+...
+
diff --git a/MAINTAINERS b/MAINTAINERS
index d516295978a4..3ed81a8fd1d0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2042,7 +2042,7 @@  F:	Documentation/devicetree/bindings/arm/arm,trace-buffer-extension.yaml
 F:	Documentation/devicetree/bindings/arm/qcom,coresight-*.yaml
 F:	Documentation/trace/coresight/*
 F:	drivers/hwtracing/coresight/*
-F:	include/dt-bindings/arm/coresight-cti-dt.h
+F:	include/dt-bindings/arm/coresight-*.h
 F:	include/linux/coresight*
 F:	samples/coresight/*
 F:	tools/perf/arch/arm/util/auxtrace.c
diff --git a/include/dt-bindings/arm/coresight-csr-dt.h b/include/dt-bindings/arm/coresight-csr-dt.h
new file mode 100644
index 000000000000..804b9bbeb2bd
--- /dev/null
+++ b/include/dt-bindings/arm/coresight-csr-dt.h
@@ -0,0 +1,12 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * This header provides constants for the defined device
+ * types on CoreSight CSR.
+ */
+
+#ifndef _DT_BINDINGS_ARM_CORESIGHT_CSR_DT_H
+#define _DT_BINDINGS_ARM_CORESIGHT_CSR_DT_H
+
+#define CSR_ASSOC_DEV_ETR	1
+
+#endif /*_DT_BINDINGS_ARM_CORESIGHT_CSR_DT_H */