lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20210125152122.GA348771@robh.at.kernel.org>
Date:   Mon, 25 Jan 2021 09:21:22 -0600
From:   Rob Herring <robh@...nel.org>
To:     alexandru.tachici@...log.com
Cc:     linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, sboyd@...nel.org,
        mturquette@...libre.com, petre.minciunescu@...log.com
Subject: Re: [PATCH 2/2] dt-bindings: clock: ad9545: Add documentation

On Mon, Jan 25, 2021 at 12:13:04AM +0200, alexandru.tachici@...log.com wrote:
> From: Alexandru Tachici <alexandru.tachici@...log.com>
> 
> Add dt bindings for ad9545.

This is kind of verbose just for a clock chip...

Maybe some of the configuration should be encoded into the clock cells 
at least where the config is determined by the consumer of a clock.

> Signed-off-by: Alexandru Tachici <alexandru.tachici@...log.com>
> ---
>  .../devicetree/bindings/clock/clk-ad9545.yaml | 352 ++++++++++++++++++
>  1 file changed, 352 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/clk-ad9545.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/clk-ad9545.yaml b/Documentation/devicetree/bindings/clock/clk-ad9545.yaml
> new file mode 100644
> index 000000000000..3611eaaa145c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/clk-ad9545.yaml
> @@ -0,0 +1,352 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/clk-ad9545.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD9545 Quad Input, 10-Output, Dual DPLL/IEEE 1588
> +
> +maintainers:
> +  - Alexandru Tachici <alexandru.tachici@...log.com>
> +
> +description: |
> +  Analog Devices AD9545 Quad Input, 10-Output, Dual DPLL/IEEE 1588,
> +  1 pps Synchronizer and Jitter Cleaner
> +
> +  Relevant documents:
> +    [1] ad9545 datasheet:
> +      https://www.analog.com/media/en/technical-documentation/data-sheets/ad9545.pdf
> +
> +    [2] Defines used: include/dt-bindings/clock/ad9545.h
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad9545
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  "#clock-cells":
> +    description: |
> +      The first cell indicates the clock type and the second number is the clock address (see [2]).
> +    const: 2
> +
> +  reg:
> +    description: |
> +      I2C address of the secondary device.
> +    minimum: 0
> +    maximum: 0xFF

Isn't 0x7f the max? In any case, that's something for the I2C bus schema 
to define. If you don't have specific addresses (I doubt all addresses 
are possible), then just:

reg:
  maxItems: 1

> +
> +  avcc-supply:
> +    description: |
> +      Phandle to the Avcc power supply.
> +
> +  adi,freq-doubler:
> +    description: |
> +      The system clock PLL provides the user with the option of doubling the reference frequency.
> +    type: boolean
> +
> +  adi,ref-crystal:
> +    description: |
> +      At XOA,XOB there is a crystal connected that needs maintaining.
> +      Otherwise it is assumed that there is a TCXO or OCXO connected.
> +    type: boolean
> +
> +  adi,ref-frequency-hz:
> +    description: |
> +      Reference input frequency at XOA,XOB. This is used for the system clock.
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32

Don't need a type for standard units.

> +
> +  clocks:
> +    items:
> +      - description: Ref A clock input
> +      - description: Ref AA clock input
> +      - description: Ref B clock input
> +      - description: Ref BB clock input
> +    maxItems: 4
> +
> +  clock-output-names:
> +    minItems: 1
> +    maxItems: 10
> +
> +  assigned-clocks:
> +     minItems: 1
> +     maxItems: 14
> +
> +  assigned-clock-rates:
> +    description: |
> +      Can initialize frequency of the 2 auxiliary NCOs, 2 PLLs and 10 clock outputs.
> +      Output clock rates must be one of the divisors of PLL assigned frequency.
> +    minItems: 1
> +    maxItems: 14
> +
> +patternProperties:
> +  "^ref-input-clk@[0-3]$":
> +    description: |
> +      Represents a reference clock input.
> +    type: object
> +
> +    properties:
> +      reg:
> +        description: |
> +          The reference input number. It can have up to 4 input clocks numbered from 0 to 3.
> +          (mapped: [refa, refaa, refb, refbb] -> [0, 1, 2, 3])
> +        maxItems: 1
> +
> +      adi,single-ended-mode:
> +        description: |
> +          Single-ended configuration mode.
> +        allOf:

Don't need allOf.

> +          - $ref: /schemas/types.yaml#/definitions/uint32
> +          - enum: [0, 1, 2, 3]
> +        maxItems: 1

'maxItems' is for an array. This is a scalar.

> +
> +      adi,differential-mode:
> +        description: |
> +          Differential configuration mode.
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32
> +          - enum: [0, 1, 2]
> +        maxItems: 1
> +
> +      adi,r-divider-ratio:
> +        description: |
> +          Each reference input has a dedicated divider.
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32
> +          - minimum: 1
> +          - maximum: 1073741824
> +        maxItems: 1
> +
> +      adi,ref-dtol-pbb:
> +        description: |
> +          REFx offset limit. Constitutes a fractional portion of the corresponding nominal period.
> +          The 24-bit number represents fractional units of parts per billion (ppb) up to a
> +          maximum of approximately 17 million ppb (1.7%).
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32
> +          - minimum: 0
> +          - maximum: 16777215
> +          - default: 100000
> +
> +      adi,ref-monitor-hysteresis-pbb:
> +        description: |
> +          Basis points of the offset limit representing per ten thousand of REFx offset limit.
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32
> +          - enum: [0, 3125, 6250, 12500, 25000, 50000, 75000, 87500]
> +          - default: 12500
> +
> +      adi,ref-validation-timer-ms:
> +        description: |
> +          Time required for a reference to remain in tolerance condition before being
> +          available to be used.
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32
> +          - minimum: 1
> +          - maximum: 1048574
> +          - default: 10
> +
> +      adi,freq-lock-threshold-ps:
> +        description: |
> +          Phase lock detector threshold (in picoseconds).
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32
> +          - minimum: 1
> +          - maximum: 16777215
> +        maxItems: 1
> +
> +      adi,phase-lock-threshold-ps:
> +        description: |
> +          Profile 0 frequency lock threshold. Frequency lock detector threshold (in picoseconds).
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32
> +          - minimum: 1
> +          - maximum: 16777215
> +        maxItems: 1
> +
> +    required:
> +      - reg
> +      - adi,r-divider-ratio
> +      - adi,ref-dtol-pbb
> +      - adi,ref-monitor-hysteresis-pbb
> +      - adi,ref-validation-timer-ms
> +      - adi,freq-lock-threshold-ps
> +      - adi,phase-lock-threshold-ps
> +
> +  "^pll-clk@[0-1]$":

You already used addresses 0-1 above. There should only be 1 address 
space at a given level.

> +    description: |
> +      Represents a PLL.
> +    type: object
> +
> +    properties:
> +      reg:
> +        description: |
> +          PLL number. AD9545 has two PLLs.
> +        maxItems: 1
> +
> +      adi,pll-source:
> +        description: |
> +          Each PLL can have 1 signal source. Choose from Ref-A to Ref-BB [0-3] or aux NCOs [4-5].

Sounds like what the assigned-clocks binding is for...

> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32
> +          - enum: [0, 1, 2, 3, 4, 5]
> +        maxItems: 1
> +
> +      adi,pll-loop-bandwidth-hz:
> +        description: |
> +          PLL loop bandwidth.
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32
> +          - minimum: 1
> +          - maximum: 1850
> +        maxItems: 1
> +
> +    required:
> +      - reg
> +      - adi,pll-source
> +      - adi,pll-loop-bandwidth-hz
> +
> +  "^aux-nco-clk@[0-1]$":
> +    description: |
> +      Represents an auxiliary Numerical Controlled Oscilator. Generates timestamps that
> +      can be sent to the DPLL0 or DPLL1.
> +    type: object
> +
> +    properties:
> +      reg:
> +        description: |
> +          Auxiliary NCO address (see [2]).
> +        maxItems: 1
> +
> +      adi,freq-lock-threshold-ps:
> +        description: |
> +          Phase lock detector threshold (in picoseconds).
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32
> +          - minimum: 1
> +          - maximum: 16777215
> +        maxItems: 1
> +
> +      adi,phase-lock-threshold-ps:
> +        description: |
> +          Profile 0 frequency lock threshold. Frequency lock detector threshold (in picoseconds).
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32
> +          - minimum: 1
> +          - maximum: 16777215
> +        maxItems: 1
> +
> +    required:
> +      - reg
> +      - adi,freq-lock-threshold-ps
> +      - adi,phase-lock-threshold-ps
> +
> +  "^output-clk@([0-9])$":
> +    description: |
> +      Represents a clock output.
> +    type: object
> +
> +    properties:
> +      reg:
> +        description: |
> +          The reference input number. It can have up to 10 output clocks mapped:
> +          (OUT0AP OUT0AN OUT0BP OUT0BN OUT0CP OUT0CN OUT1AP OUT1AN OUT1BP OUT1BN) ->
> +          (0, 1, 2, 3, 4, 5, 6, 7, 8, 9)
> +        maxItems: 1
> +
> +      adi,current-source:
> +        description: |
> +          If specified output is set as current source.
> +        type: boolean
> +
> +      adi,current-source-microamp:
> +        description: |
> +          The magnitude of the driver current.
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32
> +          - enum: [7500, 12500, 15000]
> +        minItems: 1
> +
> +      adi,output-mode:
> +        description: |
> +          Output driver mode.
> +        allOf:
> +          - $ref: /schemas/types.yaml#/definitions/uint32
> +          - enum: [0, 1, 2]
> +        maxItems: 1
> +
> +    required:
> +      - reg
> +      - adi,current-source-microamp
> +      - adi,output-mode
> +
> +  clocks: true
> +  assigned-clocks: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - adi,ref-frequency-hz
> +
> +"additionalProperties": false

Don't need quotes.

> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/ad9545.h>
> +
> +    i2c1 {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            ad9545_clock: ad9545@4A {
> +                compatible = "adi,ad9545";
> +                reg = <0x4A>;
> +
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                adi,ref-crystal;
> +                adi,ref-frequency-hz = <49152000>;
> +
> +                #clock-cells = <2>;
> +                clocks = <&ad9545_clock AD9545_CLK_NCO AD9545_NCO0>,
> +                         <&ad9545_clock AD9545_CLK_PLL AD9545_PLL1>,
> +                         <&ad9545_clock AD9545_CLK_OUT AD9545_Q1A>,
> +                         <&ad9545_clock AD9545_CLK_OUT AD9545_Q1B>;
> +                assigned-clocks = <&ad9545_clock AD9545_CLK_NCO AD9545_NCO0>,
> +                                  <&ad9545_clock AD9545_CLK_PLL AD9545_PLL1>,
> +                                  <&ad9545_clock AD9545_CLK_OUT AD9545_Q1A>,
> +                                  <&ad9545_clock AD9545_CLK_OUT AD9545_Q1B>;
> +                assigned-clock-rates = <10000>, <1875000000>, <156250000>, <156250000>;
> +
> +                aux-nco-clk@...545_NCO0 {
> +                        reg = <AD9545_NCO0>;
> +                        adi,freq-lock-threshold-ps = <16000000>;
> +                        adi,phase-lock-threshold-ps = <16000000>;
> +                };
> +
> +                ad9545_apll1: pll-clk@...545_PLL1 {
> +                        reg = <AD9545_PLL1>;
> +                        adi,pll-source = <4>;
> +                        adi,pll-loop-bandwidth-hz = <200>;
> +                };
> +
> +                output-clk@...545_Q1A {
> +                        reg = <AD9545_Q1A>;
> +                        adi,output-mode = <DRIVER_MODE_SINGLE_DIV_DIF>;
> +                        adi,current-source-microamp = <15000>;
> +                };
> +
> +                output-clk@...545_Q1B {
> +                        reg = <AD9545_Q1B>;
> +                        adi,output-mode = <DRIVER_MODE_SINGLE_DIV_DIF>;
> +                        adi,current-source-microamp = <15000>;
> +                };
> +            };
> +    };
> +...
> -- 
> 2.20.1
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ