Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 4 May 2020 23:04:19 -0500
From:   Rob Herring <robh@kernel.org>
To:     Grygorii Strashko <grygorii.strashko@ti.com>
Cc:     Richard Cochran <richardcochran@gmail.com>,
        Murali Karicheri <m-karicheri2@ti.com>,
        "David S. Miller" <davem@davemloft.net>,
        Tero Kristo <t-kristo@ti.com>,
        Lokesh Vutla <lokeshvutla@ti.com>, netdev@vger.kernel.org,
        Sekhar Nori <nsekhar@ti.com>, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        Nishanth Menon <nm@ti.com>
Subject: Re: [PATCH net-next 1/7] dt-binding: ti: am65x: document common
 platform time sync cpts module
Message-ID: <20200505040419.GA8509@bogus>
References: <20200501205011.14899-1-grygorii.strashko@ti.com>
 <20200501205011.14899-2-grygorii.strashko@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200501205011.14899-2-grygorii.strashko@ti.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, May 01, 2020 at 11:50:05PM +0300, Grygorii Strashko wrote:
> Document device tree bindings for TI AM654/J721E SoC The Common Platform
> Time Sync (CPTS) module. The CPTS module is used to facilitate host control
> of time sync operations. Main features of CPTS module are:
>   - selection of multiple external clock sources
>   - 64-bit timestamp mode in ns with ppm and nudge adjustment.
>   - control of time sync events via interrupt or polling
>   - hardware timestamp of ext. events (HWx_TS_PUSH)
>   - periodic generator function outputs (TS_GENFx)
>   - PPS in combination with timesync router
>   - Depending on integration it enables compliance with the IEEE 1588-2008
> standard for a precision clock synchronization protocol, Ethernet Enhanced
> Scheduled Traffic Operations (CPTS_ESTFn) and PCIe Subsystem Precision Time
> Measurement (PTM).
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   |   7 +
>  .../bindings/net/ti,k3-am654-cpts.yaml        | 152 ++++++++++++++++++
>  2 files changed, 159 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/ti,k3-am654-cpts.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> index 78bf511e2892..0f3fde45e200 100644
> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> @@ -144,6 +144,13 @@ patternProperties:
>      description:
>        CPSW MDIO bus.
>  
> +  "^cpts$":

Fixed strings go under 'properties'.

> +    type: object
> +    allOf:
> +      - $ref: "ti,am654-cpts.yaml#"
> +    description:
> +      CPSW Common Platform Time Sync (CPTS) module.
> +
>  required:
>    - compatible
>    - reg
> diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpts.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpts.yaml
> new file mode 100644
> index 000000000000..1b535d41e5c6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpts.yaml
> @@ -0,0 +1,152 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/ti,am654-cpts.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: The TI AM654x/J721E Common Platform Time Sync (CPTS) module Device Tree Bindings
> +
> +maintainers:
> +  - Grygorii Strashko <grygorii.strashko@ti.com>
> +  - Sekhar Nori <nsekhar@ti.com>
> +
> +description: |+
> +  The TI AM654x/J721E CPTS module is used to facilitate host control of time
> +  sync operations.
> +  Main features of CPTS module are
> +  - selection of multiple external clock sources
> +  - Software control of time sync events via interrupt or polling
> +  - 64-bit timestamp mode in ns with PPM and nudge adjustment.
> +  - hardware timestamp push inputs (HWx_TS_PUSH)
> +  - timestamp counter compare output (TS_COMP)
> +  - timestamp counter bit output (TS_SYNC)
> +  - periodic Generator function outputs (TS_GENFx)
> +  - Ethernet Enhanced Scheduled Traffic Operations (CPTS_ESTFn) (TSN)
> +  - external hardware timestamp push inputs (HWx_TS_PUSH) timestamping
> +
> +   Depending on integration it enables compliance with the IEEE 1588-2008
> +   standard for a precision clock synchronization protocol, Ethernet Enhanced
> +   Scheduled Traffic Operations (CPTS_ESTFn) and PCIe Subsystem Precision Time
> +   Measurement (PTM).
> +
> +  TI AM654x/J721E SoCs has several similar CPTS modules integrated into the
> +  different parts of the system which could be synchronized with each other
> +  - Main CPTS
> +  - MCU CPSW CPTS with IEEE 1588-2008 support
> +  - PCIe subsystem CPTS for PTM support
> +
> +  Depending on CPTS module integration and when CPTS is integral part of
> +  another module (MCU CPSW for example) "compatible" and "reg" can
> +  be omitted - parent module is fully responsible for CPTS enabling and
> +  configuration.

That's fine, but you should still have compatible and reg.

> +
> +properties:
> +  $nodename:
> +    pattern: "^cpts(@.*|-[0-9a-f])*$"
> +
> +  compatible:
> +    oneOf:
> +      - const: ti,am65-cpts
> +      - const: ti,j721e-cpts
> +
> +  reg:
> +    maxItems: 1
> +    description:
> +       The physical base address and size of CPTS IO range

Wrong indentation.

> +
> +  reg-names:
> +    items:
> +      - const: cpts

Don't really need *-names when there's only one and you haven't picked 
very meaningful names.

> +
> +  clocks:
> +    description: CPTS reference clock
> +
> +  clock-names:
> +    items:
> +      - const: cpts
> +
> +  interrupts-extended:

Use 'interrupts' here, the tooling will fixup things to allow both.

> +    items:
> +      - description: CPTS events interrupt
> +
> +  interrupt-names:
> +    items:
> +      - const: "cpts"

Don't need quotes.

> +
> +  ti,cpts-ext-ts-inputs:
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    maximum: 8
> +    description:
> +        Number of hardware timestamp push inputs (HWx_TS_PUSH)

Wrong indentation.

> +
> +  ti,cpts-periodic-outputs:
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    maximum: 8
> +    description:
> +         Number of timestamp Generator function outputs (TS_GENFx)

Wrong indentation.

> +
> +  refclk-mux:
> +    type: object
> +    description: CPTS reference clock multiplexer clock
> +    properties:
> +      '#clock-cells':
> +        const: 0
> +
> +      clocks:
> +        maxItems: 8
> +
> +      assigned-clocks:
> +        maxItems: 1
> +
> +      assigned-clocks-parents:
> +        maxItems: 1
> +
> +    required:
> +      - clocks
> +
> +required:
> +  - clocks
> +  - clock-names
> +  - interrupts-extended
> +  - interrupt-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    cpts@310d0000 {
> +         compatible = "ti,am65-cpts";
> +         reg = <0x0 0x310d0000 0x0 0x400>;
> +         reg-names = "cpts";
> +         clocks = <&main_cpts_mux>;
> +         clock-names = "cpts";
> +         interrupts-extended = <&k3_irq 163 0 IRQ_TYPE_LEVEL_HIGH>;
> +         interrupt-names = "cpts";
> +         ti,cpts-periodic-outputs = <6>;
> +         ti,cpts-ext-ts-inputs = <8>;
> +
> +         main_cpts_mux: refclk-mux {
> +               #clock-cells = <0>;
> +               clocks = <&k3_clks 118 5>, <&k3_clks 118 11>,
> +                        <&k3_clks 157 91>, <&k3_clks 157 77>,
> +                        <&k3_clks 157 102>, <&k3_clks 157 80>,
> +                        <&k3_clks 120 3>, <&k3_clks 121 3>;
> +               assigned-clocks = <&main_cpts_mux>;
> +               assigned-clock-parents = <&k3_clks 118 11>;
> +         };
> +    };
> +  - |
> +
> +    cpts {
> +             clocks = <&k3_clks 18 2>;
> +             clock-names = "cpts";
> +             interrupts-extended = <&gic500 GIC_SPI 858 IRQ_TYPE_LEVEL_HIGH>;
> +             interrupt-names = "cpts";
> +             ti,cpts-ext-ts-inputs = <4>;
> +             ti,cpts-periodic-outputs = <2>;

How is this example accessed?

> +    };
> -- 
> 2.17.1
>