[v6,05/15] dt-bindings: arm: Adds CoreSight CTI hardware definitions.

Adds new coresight-cti.yaml file describing the bindings required to define
CTI in the device trees.

Adds an include file to dt-bindings/arm to define constants describing
common signal functionality used in CoreSight and generic usage.

Signed-off-by: Mike Leach <mike.leach@linaro.org>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 .../bindings/arm/coresight-cti.yaml           | 303 ++++++++++++++++++
 .../devicetree/bindings/arm/coresight.txt     |   7 +
 MAINTAINERS                                   |   2 +
 include/dt-bindings/arm/coresight-cti-dt.h    |  37 +++
 4 files changed, 349 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/coresight-cti.yaml
 create mode 100644 include/dt-bindings/arm/coresight-cti-dt.h

Hi,

On Wed, Dec 11, 2019 at 11:08:13PM +0000, Mike Leach wrote:
> Adds new coresight-cti.yaml file describing the bindings required to define
> CTI in the device trees.
>
> Adds an include file to dt-bindings/arm to define constants describing
> common signal functionality used in CoreSight and generic usage.
>
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  .../bindings/arm/coresight-cti.yaml           | 303 ++++++++++++++++++
>  .../devicetree/bindings/arm/coresight.txt     |   7 +
>  MAINTAINERS                                   |   2 +
>  include/dt-bindings/arm/coresight-cti-dt.h    |  37 +++
>  4 files changed, 349 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/coresight-cti.yaml
>  create mode 100644 include/dt-bindings/arm/coresight-cti-dt.h
>
> diff --git a/Documentation/devicetree/bindings/arm/coresight-cti.yaml b/Documentation/devicetree/bindings/arm/coresight-cti.yaml
> new file mode 100644
> index 000000000000..cbbe88fa7148
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/coresight-cti.yaml
> @@ -0,0 +1,303 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +# Copyright 2019 Linaro Ltd.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/coresight-cti.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARM Coresight Cross Trigger Interface (CTI) device.
> +
> +description: |
> +  The CoreSight Embedded Cross Trigger (ECT) consists of CTI devices connected
> +  to one or more CoreSight components and/or a CPU, with CTIs interconnected in
> +  a star topology via the Cross Trigger Matrix (CTM), which is not programmable.
> +  The ECT components are not part of the trace generation data path and are thus
> +  not part of the CoreSight graph described in the general CoreSight bindings
> +  file coresight.txt.
> +
> +  The CTI component properties define the connections between the individual
> +  CTI and the components it is directly connected to, consisting of input and
> +  output hardware trigger signals. CTIs can have a maximum number of input and
> +  output hardware trigger signals (8 each for v1 CTI, 32 each for v2 CTI). The
> +  number is defined at design time, the maximum of each defined in the DEVID
> +  register.
> +
> +  CTIs are interconnected in a star topology via the CTM, using a number of
> +  programmable channels, usually 4, but again implementation defined and
> +  described in the DEVID register. The star topology is not required to be
> +  described in the bindings as the actual connections are software
> +  programmable.
> +
> +  In general the connections between CTI and components via the trigger signals
> +  are implementation defined, except when the CTI is connected to an ARM v8
> +  architecture core and optional ETM.
> +
> +  In this case the ARM v8 architecture defines the required signal connections
> +  between CTI and the CPU core and ETM if present. In the case of a v8
> +  architecturally connected CTI an additional compatible string is used to
> +  indicate this feature (arm,coresight-cti-v8-arch).
> +
> +  When CTI trigger connection information is unavailable then a minimal driver
> +  binding can be declared with no explicit trigger signals. This will result
> +  the driver detecting the maximum available triggers and channels from the
> +  DEVID register and make them all available for use as a single default
> +  connection. Any user / client application will require additional information
> +  on the connections between the CTI and other components for correct operation.
> +  This information might be found by enabling the Integration Test registers in
> +  the driver (set CONFIG_CORESIGHT_CTI_INTEGRATION_TEST in Kernel
> +  configuration). These registers may be used to explore the trigger connections
> +  between CTI and other CoreSight components.
> +
> +  Certain triggers between CoreSight devices and the CTI have specific types
> +  and usages. These can be defined along with the signal indexes with the
> +  constants defined in <dt-bindings/arm/coresight-cti-dt.h>
> +
> +  For example a CTI connected to a core will usually have a DBGREQ signal. This
> +  is defined in the binding as type PE_EDBGREQ. These types will appear in an
> +  optional array alongside the signal indexes. Omitting types will default all
> +  signals to GEN_IO.
> +
> +  Note that some hardware trigger signals can be connected to non-CoreSight
> +  components (e.g. UART etc) depending on hardware implementation.
> +
> +maintainers:
> +  - Mike Leach <mike.leach@linaro.org>
> +
> +allOf:
> +  - $ref: /schemas/arm/primecell.yaml#
> +
> +# Need a custom select here or 'arm,primecell' will match on lots of nodes
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - arm,coresight-cti
> +  required:
> +    - compatible
> +
> +properties:
> +  $nodename:
> +    pattern: "^cti(@[0-9a-f]+)$"
> +  compatible:
> +    oneOf:
> +      - items:
> +        - const: arm,coresight-cti
> +        - const: arm,primecell
> +      - items:
> +        - const: arm,coresight-cti-v8-arch
> +        - const: arm,coresight-cti
> +        - const: arm,primecell
> +
> +  reg:
> +    maxItems: 1
> +
> +  cpu:
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/phandle

You can drop the allOf here, and move the $ref directly under cpu:

> +    description: Handle to cpu this device is associated with.
> +
> +  arm,cti-ctm-id:
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32

Ditto

> +    description:
> +      Defines the CTM this CTI is connected to, in large systems with multiple
> +      separate CTI/CTM nets. Typically multi-socket systems where the CTM is
> +      propagated between sockets.
> +
> +  arm,cs-dev-assoc:
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/phandle

Ditto,

> +    description:
> +      defines a phandle reference to an associated CoreSight trace device.
> +      When the associated trace device is enabled, then the respective CTI
> +      will be enabled. Use in a trig-conns node, or in CTI base node when
> +      arm,cti-v8-arch present. If the associated device has not been registered
> +      then the node name will be stored as the connection name for later
> +      resolution. If the associated device is not a CoreSight device or not
> +      registered then the node name will remain the connection name and
> +      automatic enabling will not occur.
> +
> +patternProperties:
> +  '^trig_conns@([0-9]+)$':

underscores in nodename are frowned upon (and generate a warning in
dtc), you should avoid them.

> +    type: object
> +    description:
> +      A trigger connections child node which describes the trigger signals
> +      between this CTI and another hardware device. This device may be a CPU,
> +      CoreSight device, any other hardware device or simple external IO lines.
> +      The connection may have both input and output triggers, or only one or the
> +      other.
> +
> +    properties:
> +
> +      arm,trig-in-sigs:
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> +        minItems: 1
> +        maxItems: 32
> +        description:
> +          List of CTI trigger in signal numbers in use by a trig-conns node.
> +
> +      arm,trig-in-types:
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> +        minItems: 1
> +        maxItems: 32
> +        description:
> +          List of constants representing the types for the CTI trigger in
> +          signals. Types in this array match to the corresponding signal in the
> +          arm,trig-in-sigs array. If the -types array is smaller, or omitted
> +          completely, then the types will default to GEN_IO.
> +
> +      arm,trig-out-sigs:
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> +        minItems: 1
> +        maxItems: 32
> +        description:
> +          List of CTI trigger out signal numbers in use by a trig-conns node.
> +
> +      arm,trig-out-types:
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> +        minItems: 1
> +        maxItems: 32
> +        description:
> +          List of constants representing the types for the CTI trigger out
> +          signals. Types in this array match to the corresponding signal
> +          in the arm,trig-out-sigs array. If the "-types" array is smaller,
> +          or omitted completely, then the types will default to GEN_IO.
> +
> +      arm,trig-filters:
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> +        minItems: 1
> +        maxItems: 32
> +        description:
> +          List of CTI trigger out signals that will be blocked from becoming
> +          active, unless filtering is disabled on the driver.
> +
> +      arm,trig-conn-name:
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/string
> +        description:
> +          Defines a connection name that will be displayed, if the cpu or
> +          arm,cs-dev-assoc properties are not being used in this connection.
> +          Principle use for CTI that are connected to non-CoreSight devices, or
> +          external IO.
> +
> +    anyOf:
> +      - required:
> +        - arm,trig-in-sigs
> +      - required:
> +        - arm,trig-out-sigs
> +    oneOf:
> +      - required:
> +        - arm,trig-conn-name
> +      - required:
> +        - cpu
> +      - required:
> +        - arm,cs-dev-assoc

This would mean that either arm,trig-conn-name, cpu xor
arm,cs-dev-assoc is required in the trig_conn child nodes, but they
don't seem to be defined at this level but in the parent node?

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +
> +examples:
> +  # minimum CTI definition. DEVID register used to set number of triggers.
> +  - |
> +    cti@20020000 {
> +      compatible = "arm,coresight-cti", "arm,primecell";
> +      reg = <0x20020000 0x1000>;
> +
> +      clocks = <&soc_smc50mhz>;
> +      clock-names = "apb_pclk";
> +    };
> +  #  v8 architecturally defined CTI - CPU + ETM connections generated by the
> +  #  driver according to the v8 architecture specification.
> +  - |
> +    cti@859000 {
> +      compatible = "arm,coresight-cti-v8-arch", "arm,coresight-cti",
> +                   "arm,primecell";
> +      reg = <0x859000 0x1000>;
> +
> +      clocks = <&soc_smc50mhz>;
> +      clock-names = "apb_pclk";
> +
> +      cpu = <&CPU1>;
> +      arm,cs-dev-assoc = <&etm1>;

and it looks like arm,cs-dev-assoc and cpu can be combined?

> +    };
> +  # Implementation defined CTI - CPU + ETM connections explicitly defined..
> +  # Shows use of type constants from dt-bindings/arm/coresight-cti-dt.h
> +  - |
> +    #include <dt-bindings/arm/coresight-cti-dt.h>
> +
> +    cti@858000 {
> +      compatible = "arm,coresight-cti", "arm,primecell";
> +      reg = <0x858000 0x1000>;
> +
> +      clocks = <&soc_smc50mhz>;
> +      clock-names = "apb_pclk";
> +
> +      arm,cti-ctm-id = <1>;
> +
> +      trig-conns@0 {

So, what I think happened here is that your patternProperties doesn't
match anything (underscore vs dash), and since you don't have
additionalProperties set to false, it doesn't warn that there's
properties that aren't defined in the binding. Meaning that none of
the child nodes in the bindings are effectively validated.

> +            arm,trig-in-sigs = <4 5 6 7>;
> +            arm,trig-in-types = <ETM_EXTOUT
> +                                 ETM_EXTOUT
> +                                 ETM_EXTOUT
> +                                 ETM_EXTOUT>;
> +            arm,trig-out-sigs = <4 5 6 7>;
> +            arm,trig-out-types = <ETM_EXTIN
> +                                  ETM_EXTIN
> +                                  ETM_EXTIN
> +                                  ETM_EXTIN>;
> +            arm,cs-dev-assoc = <&etm0>;
> +      };

Nodes with unit-address must have a matching reg property. This will
generate a dtc warning too.

Maxime

Hi Maxime,

Thanks for the feedback.

On Fri, 13 Dec 2019 at 11:40, Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi,
>
> On Wed, Dec 11, 2019 at 11:08:13PM +0000, Mike Leach wrote:
> > Adds new coresight-cti.yaml file describing the bindings required to define
> > CTI in the device trees.
> >
> > Adds an include file to dt-bindings/arm to define constants describing
> > common signal functionality used in CoreSight and generic usage.
> >
> > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > ---
> >  .../bindings/arm/coresight-cti.yaml           | 303 ++++++++++++++++++
> >  .../devicetree/bindings/arm/coresight.txt     |   7 +
> >  MAINTAINERS                                   |   2 +
> >  include/dt-bindings/arm/coresight-cti-dt.h    |  37 +++
> >  4 files changed, 349 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/arm/coresight-cti.yaml
> >  create mode 100644 include/dt-bindings/arm/coresight-cti-dt.h
> >
> > diff --git a/Documentation/devicetree/bindings/arm/coresight-cti.yaml b/Documentation/devicetree/bindings/arm/coresight-cti.yaml
> > new file mode 100644
> > index 000000000000..cbbe88fa7148
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/coresight-cti.yaml
> > @@ -0,0 +1,303 @@
> > +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> > +# Copyright 2019 Linaro Ltd.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/arm/coresight-cti.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ARM Coresight Cross Trigger Interface (CTI) device.
> > +
> > +description: |
> > +  The CoreSight Embedded Cross Trigger (ECT) consists of CTI devices connected
> > +  to one or more CoreSight components and/or a CPU, with CTIs interconnected in
> > +  a star topology via the Cross Trigger Matrix (CTM), which is not programmable.
> > +  The ECT components are not part of the trace generation data path and are thus
> > +  not part of the CoreSight graph described in the general CoreSight bindings
> > +  file coresight.txt.
> > +
> > +  The CTI component properties define the connections between the individual
> > +  CTI and the components it is directly connected to, consisting of input and
> > +  output hardware trigger signals. CTIs can have a maximum number of input and
> > +  output hardware trigger signals (8 each for v1 CTI, 32 each for v2 CTI). The
> > +  number is defined at design time, the maximum of each defined in the DEVID
> > +  register.
> > +
> > +  CTIs are interconnected in a star topology via the CTM, using a number of
> > +  programmable channels, usually 4, but again implementation defined and
> > +  described in the DEVID register. The star topology is not required to be
> > +  described in the bindings as the actual connections are software
> > +  programmable.
> > +
> > +  In general the connections between CTI and components via the trigger signals
> > +  are implementation defined, except when the CTI is connected to an ARM v8
> > +  architecture core and optional ETM.
> > +
> > +  In this case the ARM v8 architecture defines the required signal connections
> > +  between CTI and the CPU core and ETM if present. In the case of a v8
> > +  architecturally connected CTI an additional compatible string is used to
> > +  indicate this feature (arm,coresight-cti-v8-arch).
> > +
> > +  When CTI trigger connection information is unavailable then a minimal driver
> > +  binding can be declared with no explicit trigger signals. This will result
> > +  the driver detecting the maximum available triggers and channels from the
> > +  DEVID register and make them all available for use as a single default
> > +  connection. Any user / client application will require additional information
> > +  on the connections between the CTI and other components for correct operation.
> > +  This information might be found by enabling the Integration Test registers in
> > +  the driver (set CONFIG_CORESIGHT_CTI_INTEGRATION_TEST in Kernel
> > +  configuration). These registers may be used to explore the trigger connections
> > +  between CTI and other CoreSight components.
> > +
> > +  Certain triggers between CoreSight devices and the CTI have specific types
> > +  and usages. These can be defined along with the signal indexes with the
> > +  constants defined in <dt-bindings/arm/coresight-cti-dt.h>
> > +
> > +  For example a CTI connected to a core will usually have a DBGREQ signal. This
> > +  is defined in the binding as type PE_EDBGREQ. These types will appear in an
> > +  optional array alongside the signal indexes. Omitting types will default all
> > +  signals to GEN_IO.
> > +
> > +  Note that some hardware trigger signals can be connected to non-CoreSight
> > +  components (e.g. UART etc) depending on hardware implementation.
> > +
> > +maintainers:
> > +  - Mike Leach <mike.leach@linaro.org>
> > +
> > +allOf:
> > +  - $ref: /schemas/arm/primecell.yaml#
> > +
> > +# Need a custom select here or 'arm,primecell' will match on lots of nodes
> > +select:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - arm,coresight-cti
> > +  required:
> > +    - compatible
> > +
> > +properties:
> > +  $nodename:
> > +    pattern: "^cti(@[0-9a-f]+)$"
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +        - const: arm,coresight-cti
> > +        - const: arm,primecell
> > +      - items:
> > +        - const: arm,coresight-cti-v8-arch
> > +        - const: arm,coresight-cti
> > +        - const: arm,primecell
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  cpu:
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/phandle
>
> You can drop the allOf here, and move the $ref directly under cpu:
>

OK - I'll fixup whenever this occurs.

> > +    description: Handle to cpu this device is associated with.
> > +
> > +  arm,cti-ctm-id:
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/uint32
>
> Ditto
>
> > +    description:
> > +      Defines the CTM this CTI is connected to, in large systems with multiple
> > +      separate CTI/CTM nets. Typically multi-socket systems where the CTM is
> > +      propagated between sockets.
> > +
> > +  arm,cs-dev-assoc:
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/phandle
>
> Ditto,
>
> > +    description:
> > +      defines a phandle reference to an associated CoreSight trace device.
> > +      When the associated trace device is enabled, then the respective CTI
> > +      will be enabled. Use in a trig-conns node, or in CTI base node when
> > +      arm,cti-v8-arch present. If the associated device has not been registered
> > +      then the node name will be stored as the connection name for later
> > +      resolution. If the associated device is not a CoreSight device or not
> > +      registered then the node name will remain the connection name and
> > +      automatic enabling will not occur.
> > +
> > +patternProperties:
> > +  '^trig_conns@([0-9]+)$':
>
> underscores in nodename are frowned upon (and generate a warning in
> dtc), you should avoid them.
>

This is an error on my part - trig-conns is in fact what is used in
the bindings and handling code.

> > +    type: object
> > +    description:
> > +      A trigger connections child node which describes the trigger signals
> > +      between this CTI and another hardware device. This device may be a CPU,
> > +      CoreSight device, any other hardware device or simple external IO lines.
> > +      The connection may have both input and output triggers, or only one or the
> > +      other.
> > +
> > +    properties:
> > +
> > +      arm,trig-in-sigs:
> > +        allOf:
> > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > +        minItems: 1
> > +        maxItems: 32
> > +        description:
> > +          List of CTI trigger in signal numbers in use by a trig-conns node.
> > +
> > +      arm,trig-in-types:
> > +        allOf:
> > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > +        minItems: 1
> > +        maxItems: 32
> > +        description:
> > +          List of constants representing the types for the CTI trigger in
> > +          signals. Types in this array match to the corresponding signal in the
> > +          arm,trig-in-sigs array. If the -types array is smaller, or omitted
> > +          completely, then the types will default to GEN_IO.
> > +
> > +      arm,trig-out-sigs:
> > +        allOf:
> > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > +        minItems: 1
> > +        maxItems: 32
> > +        description:
> > +          List of CTI trigger out signal numbers in use by a trig-conns node.
> > +
> > +      arm,trig-out-types:
> > +        allOf:
> > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > +        minItems: 1
> > +        maxItems: 32
> > +        description:
> > +          List of constants representing the types for the CTI trigger out
> > +          signals. Types in this array match to the corresponding signal
> > +          in the arm,trig-out-sigs array. If the "-types" array is smaller,
> > +          or omitted completely, then the types will default to GEN_IO.
> > +
> > +      arm,trig-filters:
> > +        allOf:
> > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > +        minItems: 1
> > +        maxItems: 32
> > +        description:
> > +          List of CTI trigger out signals that will be blocked from becoming
> > +          active, unless filtering is disabled on the driver.
> > +
> > +      arm,trig-conn-name:
> > +        allOf:
> > +          - $ref: /schemas/types.yaml#/definitions/string
> > +        description:
> > +          Defines a connection name that will be displayed, if the cpu or
> > +          arm,cs-dev-assoc properties are not being used in this connection.
> > +          Principle use for CTI that are connected to non-CoreSight devices, or
> > +          external IO.
> > +
> > +    anyOf:
> > +      - required:
> > +        - arm,trig-in-sigs
> > +      - required:
> > +        - arm,trig-out-sigs
> > +    oneOf:
> > +      - required:
> > +        - arm,trig-conn-name
> > +      - required:
> > +        - cpu
> > +      - required:
> > +        - arm,cs-dev-assoc
>
> This would mean that either arm,trig-conn-name, cpu xor
> arm,cs-dev-assoc is required in the trig_conn child nodes, but they
> don't seem to be defined at this level but in the parent node?
>

cpu and rm,cs-dev-assoc can appear in the parent node if the
arm,coresight-cti-v8-arch compatible string is used - hence declared
at that level. See the examples for the v8 compatible layout.

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +
> > +examples:
> > +  # minimum CTI definition. DEVID register used to set number of triggers.
> > +  - |
> > +    cti@20020000 {
> > +      compatible = "arm,coresight-cti", "arm,primecell";
> > +      reg = <0x20020000 0x1000>;
> > +
> > +      clocks = <&soc_smc50mhz>;
> > +      clock-names = "apb_pclk";
> > +    };
> > +  #  v8 architecturally defined CTI - CPU + ETM connections generated by the
> > +  #  driver according to the v8 architecture specification.
> > +  - |
> > +    cti@859000 {
> > +      compatible = "arm,coresight-cti-v8-arch", "arm,coresight-cti",
> > +                   "arm,primecell";
> > +      reg = <0x859000 0x1000>;
> > +
> > +      clocks = <&soc_smc50mhz>;
> > +      clock-names = "apb_pclk";
> > +
> > +      cpu = <&CPU1>;
> > +      arm,cs-dev-assoc = <&etm1>;
>
> and it looks like arm,cs-dev-assoc and cpu can be combined?
>
Absolutely - a CTI can and is connected to both the CPU and an
optional ETM attached to the CPU in a v8 architecture system.

> > +    };
> > +  # Implementation defined CTI - CPU + ETM connections explicitly defined..
> > +  # Shows use of type constants from dt-bindings/arm/coresight-cti-dt.h
> > +  - |
> > +    #include <dt-bindings/arm/coresight-cti-dt.h>
> > +
> > +    cti@858000 {
> > +      compatible = "arm,coresight-cti", "arm,primecell";
> > +      reg = <0x858000 0x1000>;
> > +
> > +      clocks = <&soc_smc50mhz>;
> > +      clock-names = "apb_pclk";
> > +
> > +      arm,cti-ctm-id = <1>;
> > +
> > +      trig-conns@0 {
>
> So, what I think happened here is that your patternProperties doesn't
> match anything (underscore vs dash), and since you don't have
> additionalProperties set to false, it doesn't warn that there's
> properties that aren't defined in the binding. Meaning that none of
> the child nodes in the bindings are effectively validated.
>

OK - fixing the name should fix this.
I can't use additionalProperties as this is prohibited for bindings
that use ref: (according to the example-schema.yaml)

> > +            arm,trig-in-sigs = <4 5 6 7>;
> > +            arm,trig-in-types = <ETM_EXTOUT
> > +                                 ETM_EXTOUT
> > +                                 ETM_EXTOUT
> > +                                 ETM_EXTOUT>;
> > +            arm,trig-out-sigs = <4 5 6 7>;
> > +            arm,trig-out-types = <ETM_EXTIN
> > +                                  ETM_EXTIN
> > +                                  ETM_EXTIN
> > +                                  ETM_EXTIN>;
> > +            arm,cs-dev-assoc = <&etm0>;
> > +      };
>
> Nodes with unit-address must have a matching reg property. This will
> generate a dtc warning too.
>

That's interesting - I don't get any dtc warnings when compiling or
when running make dt_binding_checl
Is this rule documented anywhere? I cannot see it in the 0.2 spec
version from devicetree.org or any of the examples / docs in the
kernel tree.

Thanks

Mike

> Maxime

On Fri, Dec 13, 2019 at 03:04:35PM +0000, Mike Leach wrote:
> > > +    type: object
> > > +    description:
> > > +      A trigger connections child node which describes the trigger signals
> > > +      between this CTI and another hardware device. This device may be a CPU,
> > > +      CoreSight device, any other hardware device or simple external IO lines.
> > > +      The connection may have both input and output triggers, or only one or the
> > > +      other.
> > > +
> > > +    properties:
> > > +
> > > +      arm,trig-in-sigs:
> > > +        allOf:
> > > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +        minItems: 1
> > > +        maxItems: 32
> > > +        description:
> > > +          List of CTI trigger in signal numbers in use by a trig-conns node.
> > > +
> > > +      arm,trig-in-types:
> > > +        allOf:
> > > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +        minItems: 1
> > > +        maxItems: 32
> > > +        description:
> > > +          List of constants representing the types for the CTI trigger in
> > > +          signals. Types in this array match to the corresponding signal in the
> > > +          arm,trig-in-sigs array. If the -types array is smaller, or omitted
> > > +          completely, then the types will default to GEN_IO.
> > > +
> > > +      arm,trig-out-sigs:
> > > +        allOf:
> > > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +        minItems: 1
> > > +        maxItems: 32
> > > +        description:
> > > +          List of CTI trigger out signal numbers in use by a trig-conns node.
> > > +
> > > +      arm,trig-out-types:
> > > +        allOf:
> > > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +        minItems: 1
> > > +        maxItems: 32
> > > +        description:
> > > +          List of constants representing the types for the CTI trigger out
> > > +          signals. Types in this array match to the corresponding signal
> > > +          in the arm,trig-out-sigs array. If the "-types" array is smaller,
> > > +          or omitted completely, then the types will default to GEN_IO.
> > > +
> > > +      arm,trig-filters:
> > > +        allOf:
> > > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +        minItems: 1
> > > +        maxItems: 32
> > > +        description:
> > > +          List of CTI trigger out signals that will be blocked from becoming
> > > +          active, unless filtering is disabled on the driver.
> > > +
> > > +      arm,trig-conn-name:
> > > +        allOf:
> > > +          - $ref: /schemas/types.yaml#/definitions/string
> > > +        description:
> > > +          Defines a connection name that will be displayed, if the cpu or
> > > +          arm,cs-dev-assoc properties are not being used in this connection.
> > > +          Principle use for CTI that are connected to non-CoreSight devices, or
> > > +          external IO.
> > > +
> > > +    anyOf:
> > > +      - required:
> > > +        - arm,trig-in-sigs
> > > +      - required:
> > > +        - arm,trig-out-sigs
> > > +    oneOf:
> > > +      - required:
> > > +        - arm,trig-conn-name
> > > +      - required:
> > > +        - cpu
> > > +      - required:
> > > +        - arm,cs-dev-assoc
> >
> > This would mean that either arm,trig-conn-name, cpu xor
> > arm,cs-dev-assoc is required in the trig_conn child nodes, but they
> > don't seem to be defined at this level but in the parent node?
> >
>
> cpu and rm,cs-dev-assoc can appear in the parent node if the
> arm,coresight-cti-v8-arch compatible string is used - hence declared
> at that level. See the examples for the v8 compatible layout.
>
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - clocks
> > > +  - clock-names
> > > +
> > > +examples:
> > > +  # minimum CTI definition. DEVID register used to set number of triggers.
> > > +  - |
> > > +    cti@20020000 {
> > > +      compatible = "arm,coresight-cti", "arm,primecell";
> > > +      reg = <0x20020000 0x1000>;
> > > +
> > > +      clocks = <&soc_smc50mhz>;
> > > +      clock-names = "apb_pclk";
> > > +    };
> > > +  #  v8 architecturally defined CTI - CPU + ETM connections generated by the
> > > +  #  driver according to the v8 architecture specification.
> > > +  - |
> > > +    cti@859000 {
> > > +      compatible = "arm,coresight-cti-v8-arch", "arm,coresight-cti",
> > > +                   "arm,primecell";
> > > +      reg = <0x859000 0x1000>;
> > > +
> > > +      clocks = <&soc_smc50mhz>;
> > > +      clock-names = "apb_pclk";
> > > +
> > > +      cpu = <&CPU1>;
> > > +      arm,cs-dev-assoc = <&etm1>;
> >
> > and it looks like arm,cs-dev-assoc and cpu can be combined?
> >
> Absolutely - a CTI can and is connected to both the CPU and an
> optional ETM attached to the CPU in a v8 architecture system.

That's not what

> > > +    oneOf:
> > > +      - required:
> > > +        - arm,trig-conn-name
> > > +      - required:
> > > +        - cpu
> > > +      - required:
> > > +        - arm,cs-dev-assoc

Is saying though. oneOf is a xor, you can have one of the three
schemas that are valid, but not more than that.

> > > +    };
> > > +  # Implementation defined CTI - CPU + ETM connections explicitly defined..
> > > +  # Shows use of type constants from dt-bindings/arm/coresight-cti-dt.h
> > > +  - |
> > > +    #include <dt-bindings/arm/coresight-cti-dt.h>
> > > +
> > > +    cti@858000 {
> > > +      compatible = "arm,coresight-cti", "arm,primecell";
> > > +      reg = <0x858000 0x1000>;
> > > +
> > > +      clocks = <&soc_smc50mhz>;
> > > +      clock-names = "apb_pclk";
> > > +
> > > +      arm,cti-ctm-id = <1>;
> > > +
> > > +      trig-conns@0 {
> >
> > So, what I think happened here is that your patternProperties doesn't
> > match anything (underscore vs dash), and since you don't have
> > additionalProperties set to false, it doesn't warn that there's
> > properties that aren't defined in the binding. Meaning that none of
> > the child nodes in the bindings are effectively validated.
> >
>
> OK - fixing the name should fix this.
> I can't use additionalProperties as this is prohibited for bindings
> that use ref: (according to the example-schema.yaml)

Ah right, I missed that one, sorry. In this case, you can use the
keyword unevaluatedProperties instead. As its name suggests, it will
report an error if there's a property that hasn't been validated by
any schemas.

Note that this is a keyword introduced by the latest schemas spec
(draft 2019-09) which isn't supported by the DT tools yet. But putting
it into your schema will at least allow to have that check when the
tools will support it.

> > > +            arm,trig-in-sigs = <4 5 6 7>;
> > > +            arm,trig-in-types = <ETM_EXTOUT
> > > +                                 ETM_EXTOUT
> > > +                                 ETM_EXTOUT
> > > +                                 ETM_EXTOUT>;
> > > +            arm,trig-out-sigs = <4 5 6 7>;
> > > +            arm,trig-out-types = <ETM_EXTIN
> > > +                                  ETM_EXTIN
> > > +                                  ETM_EXTIN
> > > +                                  ETM_EXTIN>;
> > > +            arm,cs-dev-assoc = <&etm0>;
> > > +      };
> >
> > Nodes with unit-address must have a matching reg property. This will
> > generate a dtc warning too.
>
> That's interesting - I don't get any dtc warnings when compiling or
> when running make dt_binding_checl

Linux disables a lot of DTC warnings. You can see all (I think?) the
warnings by passing W=1 in the environment when you compile the device
trees.

> Is this rule documented anywhere? I cannot see it in the 0.2 spec
> version from devicetree.org or any of the examples / docs in the
> kernel tree.

The commit adding it to DTC is this one
https://git.kernel.org/pub/scm/utils/dtc/dtc.git/commit/?id=852e9ecbe1976927057402f8a8f71ee8e8a49098

It just seems that while valid, it's against best practices.

Maxime

Hi Maxime

On Sat, 14 Dec 2019 at 20:26, Maxime Ripard <maxime@cerno.tech> wrote:
>
> On Fri, Dec 13, 2019 at 03:04:35PM +0000, Mike Leach wrote:
> > > > +    type: object
> > > > +    description:
> > > > +      A trigger connections child node which describes the trigger signals
> > > > +      between this CTI and another hardware device. This device may be a CPU,
> > > > +      CoreSight device, any other hardware device or simple external IO lines.
> > > > +      The connection may have both input and output triggers, or only one or the
> > > > +      other.
> > > > +
> > > > +    properties:
> > > > +
> > > > +      arm,trig-in-sigs:
> > > > +        allOf:
> > > > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > +        minItems: 1
> > > > +        maxItems: 32
> > > > +        description:
> > > > +          List of CTI trigger in signal numbers in use by a trig-conns node.
> > > > +
> > > > +      arm,trig-in-types:
> > > > +        allOf:
> > > > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > +        minItems: 1
> > > > +        maxItems: 32
> > > > +        description:
> > > > +          List of constants representing the types for the CTI trigger in
> > > > +          signals. Types in this array match to the corresponding signal in the
> > > > +          arm,trig-in-sigs array. If the -types array is smaller, or omitted
> > > > +          completely, then the types will default to GEN_IO.
> > > > +
> > > > +      arm,trig-out-sigs:
> > > > +        allOf:
> > > > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > +        minItems: 1
> > > > +        maxItems: 32
> > > > +        description:
> > > > +          List of CTI trigger out signal numbers in use by a trig-conns node.
> > > > +
> > > > +      arm,trig-out-types:
> > > > +        allOf:
> > > > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > +        minItems: 1
> > > > +        maxItems: 32
> > > > +        description:
> > > > +          List of constants representing the types for the CTI trigger out
> > > > +          signals. Types in this array match to the corresponding signal
> > > > +          in the arm,trig-out-sigs array. If the "-types" array is smaller,
> > > > +          or omitted completely, then the types will default to GEN_IO.
> > > > +
> > > > +      arm,trig-filters:
> > > > +        allOf:
> > > > +          - $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > +        minItems: 1
> > > > +        maxItems: 32
> > > > +        description:
> > > > +          List of CTI trigger out signals that will be blocked from becoming
> > > > +          active, unless filtering is disabled on the driver.
> > > > +
> > > > +      arm,trig-conn-name:
> > > > +        allOf:
> > > > +          - $ref: /schemas/types.yaml#/definitions/string
> > > > +        description:
> > > > +          Defines a connection name that will be displayed, if the cpu or
> > > > +          arm,cs-dev-assoc properties are not being used in this connection.
> > > > +          Principle use for CTI that are connected to non-CoreSight devices, or
> > > > +          external IO.
> > > > +
> > > > +    anyOf:
> > > > +      - required:
> > > > +        - arm,trig-in-sigs
> > > > +      - required:
> > > > +        - arm,trig-out-sigs
> > > > +    oneOf:
> > > > +      - required:
> > > > +        - arm,trig-conn-name
> > > > +      - required:
> > > > +        - cpu
> > > > +      - required:
> > > > +        - arm,cs-dev-assoc
> > >
> > > This would mean that either arm,trig-conn-name, cpu xor
> > > arm,cs-dev-assoc is required in the trig_conn child nodes, but they
> > > don't seem to be defined at this level but in the parent node?
> > >
> >
> > cpu and rm,cs-dev-assoc can appear in the parent node if the
> > arm,coresight-cti-v8-arch compatible string is used - hence declared
> > at that level. See the examples for the v8 compatible layout.
> >
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - clocks
> > > > +  - clock-names
> > > > +
> > > > +examples:
> > > > +  # minimum CTI definition. DEVID register used to set number of triggers.
> > > > +  - |
> > > > +    cti@20020000 {
> > > > +      compatible = "arm,coresight-cti", "arm,primecell";
> > > > +      reg = <0x20020000 0x1000>;
> > > > +
> > > > +      clocks = <&soc_smc50mhz>;
> > > > +      clock-names = "apb_pclk";
> > > > +    };
> > > > +  #  v8 architecturally defined CTI - CPU + ETM connections generated by the
> > > > +  #  driver according to the v8 architecture specification.
> > > > +  - |
> > > > +    cti@859000 {
> > > > +      compatible = "arm,coresight-cti-v8-arch", "arm,coresight-cti",
> > > > +                   "arm,primecell";
> > > > +      reg = <0x859000 0x1000>;
> > > > +
> > > > +      clocks = <&soc_smc50mhz>;
> > > > +      clock-names = "apb_pclk";
> > > > +
> > > > +      cpu = <&CPU1>;
> > > > +      arm,cs-dev-assoc = <&etm1>;
> > >
> > > and it looks like arm,cs-dev-assoc and cpu can be combined?
> > >
> > Absolutely - a CTI can and is connected to both the CPU and an
> > optional ETM attached to the CPU in a v8 architecture system.
>
> That's not what
>
> > > > +    oneOf:
> > > > +      - required:
> > > > +        - arm,trig-conn-name
> > > > +      - required:
> > > > +        - cpu
> > > > +      - required:
> > > > +        - arm,cs-dev-assoc
>
> Is saying though. oneOf is a xor, you can have one of the three
> schemas that are valid, but not more than that.
>

This required schema only applies to the trig-conns@ child nodes. So
each trig-conns can have one only of the three attributes - as each
trig-conns node represents a single connection between the CTI and a
component, and defines the trigger signals that exist in that
connections.

At the cti@ level, if the compatible is the arm,coresight-cti-v8-arch
type, then both cpu and arm,cs-dev-assoc can appear at this level as
we only need to know the devices it is connected to - the individual
trigger signals in the connections are defined by the architecture and
do not need repeating here.

> > > > +    };
> > > > +  # Implementation defined CTI - CPU + ETM connections explicitly defined..
> > > > +  # Shows use of type constants from dt-bindings/arm/coresight-cti-dt.h
> > > > +  - |
> > > > +    #include <dt-bindings/arm/coresight-cti-dt.h>
> > > > +
> > > > +    cti@858000 {
> > > > +      compatible = "arm,coresight-cti", "arm,primecell";
> > > > +      reg = <0x858000 0x1000>;
> > > > +
> > > > +      clocks = <&soc_smc50mhz>;
> > > > +      clock-names = "apb_pclk";
> > > > +
> > > > +      arm,cti-ctm-id = <1>;
> > > > +
> > > > +      trig-conns@0 {
> > >
> > > So, what I think happened here is that your patternProperties doesn't
> > > match anything (underscore vs dash), and since you don't have
> > > additionalProperties set to false, it doesn't warn that there's
> > > properties that aren't defined in the binding. Meaning that none of
> > > the child nodes in the bindings are effectively validated.
> > >
> >
> > OK - fixing the name should fix this.
> > I can't use additionalProperties as this is prohibited for bindings
> > that use ref: (according to the example-schema.yaml)
>
> Ah right, I missed that one, sorry. In this case, you can use the
> keyword unevaluatedProperties instead. As its name suggests, it will
> report an error if there's a property that hasn't been validated by
> any schemas.
>
> Note that this is a keyword introduced by the latest schemas spec
> (draft 2019-09) which isn't supported by the DT tools yet. But putting
> it into your schema will at least allow to have that check when the
> tools will support it.
>
> > > > +            arm,trig-in-sigs = <4 5 6 7>;
> > > > +            arm,trig-in-types = <ETM_EXTOUT
> > > > +                                 ETM_EXTOUT
> > > > +                                 ETM_EXTOUT
> > > > +                                 ETM_EXTOUT>;
> > > > +            arm,trig-out-sigs = <4 5 6 7>;
> > > > +            arm,trig-out-types = <ETM_EXTIN
> > > > +                                  ETM_EXTIN
> > > > +                                  ETM_EXTIN
> > > > +                                  ETM_EXTIN>;
> > > > +            arm,cs-dev-assoc = <&etm0>;
> > > > +      };
> > >
> > > Nodes with unit-address must have a matching reg property. This will
> > > generate a dtc warning too.
> >
> > That's interesting - I don't get any dtc warnings when compiling or
> > when running make dt_binding_checl
>
> Linux disables a lot of DTC warnings. You can see all (I think?) the
> warnings by passing W=1 in the environment when you compile the device
> trees.
>
Thanks -  the W=12 starts outputting warnings for lack of reg in child nodes.
I'll build the requirement into the next version so it is explicitly
called out in the check.

> > Is this rule documented anywhere? I cannot see it in the 0.2 spec
> > version from devicetree.org or any of the examples / docs in the
> > kernel tree.
>
> The commit adding it to DTC is this one
> https://git.kernel.org/pub/scm/utils/dtc/dtc.git/commit/?id=852e9ecbe1976927057402f8a8f71ee8e8a49098

I was referring to the requirement for reg = <N> in node@<N> child
nodes - though I have now spotted this in the early part of the
0.3-rc2 spec - which appears to be the latest I have access to.

So I can add in the reg<> parameters and fix this up.

>
> It just seems that while valid, it's against best practices.
>
> Maxime

Thanks

Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

On 11/12/2019 23:08, Mike Leach wrote:
> Adds new coresight-cti.yaml file describing the bindings required to define
> CTI in the device trees.
> 
> Adds an include file to dt-bindings/arm to define constants describing
> common signal functionality used in CoreSight and generic usage.
> 
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>